Propose landing UI refresh#601
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughHeader gains JS-driven accessible dropdowns and sticky blurred styling; hero, pricing, plans and plugins UIs receive visual and accessibility tweaks; layout adds stacking/antialiasing adjustments; Spanish copyright string wording updated. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant HeaderUI as Header UI (buttons/menus)
participant Script as Header Script (state manager)
participant Document as Document
User->>HeaderUI: click/tap toggle or hover/focus
HeaderUI->>Script: dispatch toggle/open/focus event (id)
Script->>HeaderUI: update `aria-expanded` / `aria-hidden` and visibility classes
HeaderUI->>User: menu shown/hidden
User->>Document: press Escape or click outside
Document->>Script: document key/click handler
Script->>HeaderUI: close menus, update ARIA states, return focus if needed
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/web/src/components/pricing/Plans.astro (1)
90-105:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAnnounce pricing changes when billing mode toggles.
The price and billing text are updated dynamically, but there is no live region around this content, so assistive tech may not announce the change.
Suggested fix
- <div class="flex items-end"> + <div class="flex items-end" role="status" aria-live="polite" aria-atomic="true">As per coding guidelines:
**/*.{astro,jsx,tsx,js,ts}: Dynamic content must usearia-liveregions for accessibility.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/components/pricing/Plans.astro` around lines 90 - 105, The price and billing text updates aren’t announced to assistive tech; wrap the dynamic elements (the price output from priceDisplay and the blocks with data-price, data-yearly-price, data-monthly-price) in an accessible live region (e.g., a container with aria-live="polite" and aria-atomic="true" or role="status") so changes when the yearly toggle (yearly) flips are announced; ensure the month/billed strings rendered via m.month and m.billed_annually_at and the numeric output from numberWithSpaces(plan.price_y) remain inside that region and that any hidden variants use aria-hidden appropriately so only the visible text is read.apps/web/src/components/Hero.astro (1)
214-325:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDecorative feature/stat icons are exposed to assistive tech.
These SVGs are visual-only and should be hidden from the accessibility tree to avoid noisy announcements.
Suggested fix pattern (apply to all decorative SVGs in this section)
-<svg class="h-8 w-8 text-gray-400" fill="none" stroke="currentColor" viewBox="0 0 24 24"> +<svg class="h-8 w-8 text-gray-400" fill="none" stroke="currentColor" viewBox="0 0 24 24" aria-hidden="true" focusable="false">As per coding guidelines:
**/*.{astro,jsx,tsx}: Decorative SVGs must havearia-hidden="true".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/components/Hero.astro` around lines 214 - 325, Add aria-hidden="true" to all decorative SVG elements in Hero.astro so they are removed from the accessibility tree: update each <svg> with classes like "h-8 w-8 text-gray-400", the two app-store SVGs (class "h-6 w-6"), the lock/check SVG for end_to_end_encryption, and the stats icons (class "h-5 w-5 text-blue-500" and similar) to include aria-hidden="true" (and optionally focusable="false") to prevent assistive tech from announcing these purely visual icons.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/web/src/components/Header.astro`:
- Around line 49-61: The desktop flyout is currently mouse-only because the
"group" wrapper and the submenu div rely solely on Tailwind's group-hover
classes; make the products button and submenu keyboard-accessible by: (1)
turning the button (the element with aria-label={m.products(...)} and the
chevron SVG) into a proper menu toggle with aria-haspopup="true" and
aria-expanded state, (2) replace group-hover visibility rules on the submenu div
with focus/focus-within (or toggle a visible CSS class) so the submenu becomes
visible when the button or any submenu item receives focus, and (3) add minimal
keyboard handlers to the toggle (handle Enter/Space to open/close and Escape to
close) and ensure submenu links are reachable (no permanent invisible/opacity
while focused) and update aria-hidden accordingly; apply the same changes to the
other flyout block referenced around the later menu.
- Around line 33-34: The decorative SVG elements (e.g., the <svg class="size-7"
...> instances used for header icons) should be hidden from screen readers; add
aria-hidden="true" to each decorative SVG (the ones at the top-level header
icons and the other same-pattern SVGs) so they are not announced by assistive
tech.
In `@apps/web/src/pages/pricing.astro`:
- Around line 244-250: The decorative SVGs in pricing.astro (the <svg> elements
used for the premium-support icons) are currently exposed to assistive tech; add
aria-hidden="true" to each decorative <svg> (the one with class "h-10 w-10
text-orange-600" shown in the diff and the similar SVG at lines ~267-269) so
they are excluded from the accessibility tree, ensuring only meaningful content
is announced.
---
Outside diff comments:
In `@apps/web/src/components/Hero.astro`:
- Around line 214-325: Add aria-hidden="true" to all decorative SVG elements in
Hero.astro so they are removed from the accessibility tree: update each <svg>
with classes like "h-8 w-8 text-gray-400", the two app-store SVGs (class "h-6
w-6"), the lock/check SVG for end_to_end_encryption, and the stats icons (class
"h-5 w-5 text-blue-500" and similar) to include aria-hidden="true" (and
optionally focusable="false") to prevent assistive tech from announcing these
purely visual icons.
In `@apps/web/src/components/pricing/Plans.astro`:
- Around line 90-105: The price and billing text updates aren’t announced to
assistive tech; wrap the dynamic elements (the price output from priceDisplay
and the blocks with data-price, data-yearly-price, data-monthly-price) in an
accessible live region (e.g., a container with aria-live="polite" and
aria-atomic="true" or role="status") so changes when the yearly toggle (yearly)
flips are announced; ensure the month/billed strings rendered via m.month and
m.billed_annually_at and the numeric output from numberWithSpaces(plan.price_y)
remain inside that region and that any hidden variants use aria-hidden
appropriately so only the visible text is read.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: cc2b09a8-68fb-4739-90d4-b95bef83974e
⛔ Files ignored due to path filters (12)
.github/pr-assets/ui-refresh/after/home-desktop.pngis excluded by!**/*.png.github/pr-assets/ui-refresh/after/home-mobile.pngis excluded by!**/*.png.github/pr-assets/ui-refresh/after/plugins-desktop.pngis excluded by!**/*.png.github/pr-assets/ui-refresh/after/pricing-desktop.pngis excluded by!**/*.png.github/pr-assets/ui-refresh/after/solution-desktop.pngis excluded by!**/*.png.github/pr-assets/ui-refresh/after/tools-desktop.pngis excluded by!**/*.png.github/pr-assets/ui-refresh/before/home-desktop.pngis excluded by!**/*.png.github/pr-assets/ui-refresh/before/home-mobile.pngis excluded by!**/*.png.github/pr-assets/ui-refresh/before/plugins-desktop.pngis excluded by!**/*.png.github/pr-assets/ui-refresh/before/pricing-desktop.pngis excluded by!**/*.png.github/pr-assets/ui-refresh/before/solution-desktop.pngis excluded by!**/*.png.github/pr-assets/ui-refresh/before/tools-desktop.pngis excluded by!**/*.png
📒 Files selected for processing (6)
apps/web/src/components/Header.astroapps/web/src/components/Hero.astroapps/web/src/components/pricing/Plans.astroapps/web/src/layouts/Layout.astroapps/web/src/pages/plugins.astroapps/web/src/pages/pricing.astro
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: aeda446585
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/web/src/components/Header.astro`:
- Around line 362-375: Anchors in Header.astro that use target="_blank" (e.g.,
the login link that calls m.login(...) and the register link that uses
getRelativeLocaleUrl(...)) are missing rel="noopener noreferrer"; update each
<a> with target="_blank" to include rel="noopener noreferrer" to prevent
window.opener exploitation (apply the same change for the other occurrences
around lines referenced, e.g., the other anchors at ~600-610 that open new
tabs).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4ee816a5-333d-4d46-a246-5e0ddbcc9d39
📒 Files selected for processing (4)
apps/web/src/components/Header.astroapps/web/src/components/Hero.astroapps/web/src/components/pricing/Plans.astroapps/web/src/pages/pricing.astro
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/src/pages/pricing.astro
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/web/src/components/Header.astro`:
- Around line 73-74: The desktop flyout panels are initially hidden with
"invisible opacity-0" so if JS fails their links are unreachable; remove those
initial hiding classes from the desktop flyouts (refer to the elements with
id="desktop-products-menu" and the corresponding desktop solutions menu around
the other occurrences) so the menus are visible and keyboard-focusable without
JS, and instead implement the open/closed visual state purely via your script or
CSS (e.g., add/remove a JS-controlled class or use media-query-specific hiding
for mobile only); ensure the same change is applied to the other instances
called out (around lines 124-125 and 664-700) so interactive elements remain
keyboard accessible per the guideline.
- Around line 371-377: Remove the incorrect ARIA role by deleting the
role="button" attribute on the register CTA anchor element (the <a> that calls
getRelativeLocaleUrl(Astro.locals.locale, 'register') and uses m.register with
Astro.locals.locale); keep it as a normal link that opens in a new tab
(target="_blank" rel="noopener noreferrer") so assistive tech sees it as a link
instead of a button.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7f53cf12-7a83-4a23-89d5-53851b1206d0
📒 Files selected for processing (1)
apps/web/src/components/Header.astro
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b7b52a828d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7e5ed96bf0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| dropdown.wrapper.addEventListener('mouseleave', () => { | ||
| if (!dropdown.wrapper?.contains(document.activeElement)) setDesktopDropdownOpen(dropdown, false) | ||
| }) | ||
| dropdown.wrapper.addEventListener('focusin', () => setDesktopDropdownOpen(dropdown, true)) |
There was a problem hiding this comment.
The desktop dropdown wrapper always reopens on any focusin, so pressing Escape inside the menu does not actually dismiss it: the menu handler closes it and calls toggle.focus(), then the focus shift bubbles a focusin to the wrapper and setDesktopDropdownOpen(..., true) runs again. This makes Escape ineffective for keyboard users in the menu-close flow.
Useful? React with 👍 / 👎.
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bdb8afddc5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| "gray-matter": "^4.0.3", | ||
| "js-yaml": "^4.1.1", | ||
| "langchain": "^0.3.37", | ||
| "langchain": "^1.3.5", |
There was a problem hiding this comment.
This bump upgrades langchain to ^1.3.5, but repository scripts still use v0-era import paths like langchain/chains/..., langchain/chat_models/openai, and langchain/document (see scripts/generateTutorials.mjs and scripts/trainOpenAI.mjs). In LangChain v1, legacy APIs moved out of these paths (to packages such as @langchain/classic / @langchain/openai), so these scripts will fail at startup with module-resolution errors when run, breaking the tutorial/training automation workflows.
Useful? React with 👍 / 👎.
1 participant
Summary
Screenshots
1. Shared header + homepage hero
2. Mobile hero readability
3. Plugins directory density
4. Pricing card hierarchy
5. Shared header on solution pages
6. Shared header on tools pages
Validation
bunx prettier --write apps/web/src/layouts/Layout.astro apps/web/src/components/Header.astro apps/web/src/components/Hero.astro apps/web/src/components/pricing/Plans.astro apps/web/src/pages/plugins.astro apps/web/src/pages/pricing.astrobun run build:webbun run checkpassed. Web: 0 errors. Docs: 0 errors, 1 existing hint inapps/docs/src/components/doc/CopyPage.astro.Summary by CodeRabbit
Style
Bug Fixes / Accessibility
Localization