[Portal] Redesign header with improved mobile menu layout #7138
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
|
""" WalkthroughThe Header component was refactored to implement a two-row layout, separating navigation and controls for improved clarity and responsiveness. The mobile navigation was restructured into a full-screen overlay with vertical sections for categories, and the burger menu toggle was repositioned. The DocSearch component's search button width was increased, and an unused helper function was removed. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Header
participant DesktopNav
participant MobileMenu
participant BurgerMenuButton
User->>Header: Loads page
Header->>DesktopNav: Render (if screen ≥ xl)
Header->>BurgerMenuButton: Render (always visible)
User->>BurgerMenuButton: Click (on mobile)
BurgerMenuButton->>MobileMenu: Toggle visibility
User->>MobileMenu: Click navigation link
MobileMenu->>MobileMenu: Close overlay
""" Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used🧬 Code Graph Analysis (1)apps/portal/src/app/Header.tsx (2)
⏰ Context from checks skipped due to timeout of 90000ms (6)
🔇 Additional comments (3)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
github-actions
bot
added
the
Portal
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. This stack of pull requests is managed by Graphite. Learn more about stacking. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #7138 +/- ##
=======================================
Coverage 55.68% 55.68%
=======================================
Files 904 904
Lines 58324 58324
Branches 4113 4113
=======================================
Hits 32476 32476
Misses 25743 25743
Partials 105 105
🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
apps/portal/src/app/Header.tsx (1)
103-106:⚠️ Potential issue
/react-native/v5not recognised by DocSearch tag helper
connectLinksnow points to"/react-native/v5", butgetTagsFromHref()inDocSearch.tsxstill only checks for"/react-native/v0".
The new link will therefore be shown without the “React Native” tag in search results.- if (href.includes("/react-native/v0")) { + if (href.includes("/react-native/v0") || href.includes("/react-native/v5")) {Update the helper (and any related analytics) to keep tag filtering consistent.
♻️ Duplicate comments (1)
apps/portal/src/app/Header.tsx (1)
266-271: Repeat of previous comment: please addrel="noopener noreferrer"to the second GitHub link as well.
🧹 Nitpick comments (3)
apps/portal/src/components/others/DocSearch.tsx (1)
358-362: Prefer responsive width over a hard-codedw-64A fixed width of
16remcan look cramped on very small viewports and excessively wide on very large ones.
Consider allowing the button to grow with its container and clamp it only on medium+ breakpoints:- className="flex w-64 justify-between px-3" + className="flex justify-between px-3 w-full md:w-64"This keeps the current desktop appearance (
md:w-64) while letting mobile screens use the full width.apps/portal/src/app/Header.tsx (2)
175-186: DuplicateThemeSwitcherinstances can be reducedMaintaining two separate components (one hidden on
xl, the other shown onxl) duplicates state & logic.
Tailwind allows the same element to change style across breakpoints:- <div className="hidden xl:flex"> - <ThemeSwitcher /> - </div> - - <div className="flex items-center gap-1 xl:hidden"> - <ThemeSwitcher className="border-none bg-transparent" /> + <ThemeSwitcher + className="border-none bg-transparent + xl:border xl:bg-card" /* example — adjust as needed */ + />This removes duplication and guarantees both breakpoints act on the same underlying component.
188-193: Addrel="noopener noreferrer"to external links opened in a new tabFor security and performance it’s best practice to add
rel="noopener noreferrer"whenevertarget="_blank"is used:- <Link - href="https://github.com/thirdweb-dev" - target="_blank" - className="text-foreground" + <Link + href="https://github.com/thirdweb-dev" + target="_blank" + rel="noopener noreferrer" + className="text-foreground"(Apply the same change to the GitHub link in the bottom row.)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/portal/src/app/Header.tsx(2 hunks)apps/portal/src/components/others/DocSearch.tsx(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: E2E Tests (pnpm, esbuild)
- GitHub Check: E2E Tests (pnpm, webpack)
- GitHub Check: Build Packages
- GitHub Check: Size
- GitHub Check: E2E Tests (pnpm, vite)
- GitHub Check: Lint Packages
- GitHub Check: Analyze (javascript)
apps/portal/src/app/Header.tsx
Outdated
| {showBurgerMenu && ( | ||
| <div className="fixed inset-0 top-sticky-top-height z-50 overflow-auto bg-card p-6 xl:hidden"> | ||
| <div className="flex flex-col gap-6"> | ||
| <div className="flex flex-col gap-4"> | ||
| <h3 className="font-semibold text-lg">Connect</h3> | ||
| {connectLinks.map((link) => ( | ||
| <NavLink | ||
| key={link.name} | ||
| name={link.name} | ||
| href={link.href} | ||
| onClick={() => setShowBurgerMenu(false)} | ||
| icon={link.icon} | ||
| /> | ||
| ))} | ||
| </div> | ||
|
|
||
| <div className="flex flex-col gap-4"> | ||
| <h3 className="font-semibold text-lg">Products</h3> | ||
| {links.map((link) => ( | ||
| <NavLink | ||
| key={link.name} | ||
| name={link.name} | ||
| href={link.href} | ||
| onClick={() => setShowBurgerMenu(false)} | ||
| /> | ||
| ))} | ||
| </div> | ||
|
|
||
| <div className="hidden xl:block"> | ||
| <DocSearch variant="search" /> | ||
| <div className="flex flex-col gap-4"> | ||
| <h3 className="font-semibold text-lg">Tools</h3> | ||
| {toolLinks.map((link) => ( | ||
| <NavLink | ||
| key={link.name} | ||
| name={link.name} | ||
| href={link.href} | ||
| onClick={() => setShowBurgerMenu(false)} | ||
| /> | ||
| ))} | ||
| </div> | ||
|
|
||
| <div className="xl:px-1"> | ||
| <DropdownLinks | ||
| links={apisLinks} | ||
| onLinkClick={() => setShowBurgerMenu(false)} | ||
| category="APIs" | ||
| /> | ||
| <div className="flex flex-col gap-4"> | ||
| <h3 className="font-semibold text-lg">APIs</h3> | ||
| {apisLinks.map((link) => ( | ||
| <NavLink | ||
| key={link.name} | ||
| name={link.name} | ||
| href={link.href} | ||
| onClick={() => setShowBurgerMenu(false)} | ||
| /> | ||
| ))} | ||
| </div> | ||
|
|
||
| <div className="xl:px-1"> | ||
| <DropdownLinks | ||
| links={supportLinks} | ||
| onLinkClick={() => setShowBurgerMenu(false)} | ||
| category="Support" | ||
| /> | ||
| <div className="flex flex-col gap-4"> | ||
| <h3 className="font-semibold text-lg">Support</h3> | ||
| {supportLinks.map((link) => ( | ||
| <NavLink | ||
| key={link.name} | ||
| name={link.name} | ||
| href={link.href} | ||
| onClick={() => setShowBurgerMenu(false)} | ||
| /> | ||
| ))} | ||
| </div> | ||
|
|
||
| <NavLink | ||
| name="Changelog" | ||
| href="/changelog" | ||
| onClick={() => { | ||
| setShowBurgerMenu(false); | ||
| }} | ||
| onClick={() => setShowBurgerMenu(false)} | ||
| /> | ||
|
|
||
| <Link | ||
| href="https://github.com/thirdweb-dev" | ||
| target="_blank" | ||
| className="hidden text-muted-foreground transition-colors hover:text-foreground xl:block" | ||
| > | ||
| <GithubIcon className="mx-2 size-6" /> | ||
| </Link> | ||
| </div> | ||
| </nav> | ||
| </div> | ||
| </div> | ||
| )} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve accessibility & scroll-lock when the mobile menu is open
The full-screen overlay lacks:
- Focus trapping /
aria-modal="true"so keyboard users are not lost behind the overlay. - Body scroll lock – scrolling the page behind the menu may lead to context loss on close.
A minimal improvement:
- <div className="fixed inset-0 top-sticky-top-height z-50 overflow-auto bg-card p-6 xl:hidden">
+ <div
+ role="dialog"
+ aria-modal="true"
+ className="fixed inset-0 top-sticky-top-height z-50 overflow-auto bg-card p-6 xl:hidden"
+ onWheel={(e) => e.stopPropagation()} /* optional: crude scroll-lock */
+ >For a more complete solution, consider reusing your existing Dialog component (as done for DocSearch) which already handles focus-trapping and background scroll locking.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| {showBurgerMenu && ( | |
| <div className="fixed inset-0 top-sticky-top-height z-50 overflow-auto bg-card p-6 xl:hidden"> | |
| <div className="flex flex-col gap-6"> | |
| <div className="flex flex-col gap-4"> | |
| <h3 className="font-semibold text-lg">Connect</h3> | |
| {connectLinks.map((link) => ( | |
| <NavLink | |
| key={link.name} | |
| name={link.name} | |
| href={link.href} | |
| onClick={() => setShowBurgerMenu(false)} | |
| icon={link.icon} | |
| /> | |
| ))} | |
| </div> | |
| <div className="flex flex-col gap-4"> | |
| <h3 className="font-semibold text-lg">Products</h3> | |
| {links.map((link) => ( | |
| <NavLink | |
| key={link.name} | |
| name={link.name} | |
| href={link.href} | |
| onClick={() => setShowBurgerMenu(false)} | |
| /> | |
| ))} | |
| </div> | |
| <div className="hidden xl:block"> | |
| <DocSearch variant="search" /> | |
| <div className="flex flex-col gap-4"> | |
| <h3 className="font-semibold text-lg">Tools</h3> | |
| {toolLinks.map((link) => ( | |
| <NavLink | |
| key={link.name} | |
| name={link.name} | |
| href={link.href} | |
| onClick={() => setShowBurgerMenu(false)} | |
| /> | |
| ))} | |
| </div> | |
| <div className="xl:px-1"> | |
| <DropdownLinks | |
| links={apisLinks} | |
| onLinkClick={() => setShowBurgerMenu(false)} | |
| category="APIs" | |
| /> | |
| <div className="flex flex-col gap-4"> | |
| <h3 className="font-semibold text-lg">APIs</h3> | |
| {apisLinks.map((link) => ( | |
| <NavLink | |
| key={link.name} | |
| name={link.name} | |
| href={link.href} | |
| onClick={() => setShowBurgerMenu(false)} | |
| /> | |
| ))} | |
| </div> | |
| <div className="xl:px-1"> | |
| <DropdownLinks | |
| links={supportLinks} | |
| onLinkClick={() => setShowBurgerMenu(false)} | |
| category="Support" | |
| /> | |
| <div className="flex flex-col gap-4"> | |
| <h3 className="font-semibold text-lg">Support</h3> | |
| {supportLinks.map((link) => ( | |
| <NavLink | |
| key={link.name} | |
| name={link.name} | |
| href={link.href} | |
| onClick={() => setShowBurgerMenu(false)} | |
| /> | |
| ))} | |
| </div> | |
| <NavLink | |
| name="Changelog" | |
| href="/changelog" | |
| onClick={() => { | |
| setShowBurgerMenu(false); | |
| }} | |
| onClick={() => setShowBurgerMenu(false)} | |
| /> | |
| <Link | |
| href="https://github.com/thirdweb-dev" | |
| target="_blank" | |
| className="hidden text-muted-foreground transition-colors hover:text-foreground xl:block" | |
| > | |
| <GithubIcon className="mx-2 size-6" /> | |
| </Link> | |
| </div> | |
| </nav> | |
| </div> | |
| </div> | |
| )} | |
| {showBurgerMenu && ( | |
| - <div className="fixed inset-0 top-sticky-top-height z-50 overflow-auto bg-card p-6 xl:hidden"> | |
| + <div | |
| + role="dialog" | |
| + aria-modal="true" | |
| + className="fixed inset-0 top-sticky-top-height z-50 overflow-auto bg-card p-6 xl:hidden" | |
| + onWheel={(e) => e.stopPropagation()} /* optional: crude scroll-lock */ | |
| + > | |
| <div className="flex flex-col gap-6"> | |
| <div className="flex flex-col gap-4"> | |
| <h3 className="font-semibold text-lg">Connect</h3> | |
| {connectLinks.map((link) => ( | |
| <NavLink | |
| key={link.name} | |
| name={link.name} | |
| href={link.href} | |
| onClick={() => setShowBurgerMenu(false)} | |
| icon={link.icon} | |
| /> | |
| ))} | |
| </div> | |
| <div className="flex flex-col gap-4"> | |
| <h3 className="font-semibold text-lg">Products</h3> | |
| {links.map((link) => ( | |
| <NavLink | |
| key={link.name} | |
| name={link.name} | |
| href={link.href} | |
| onClick={() => setShowBurgerMenu(false)} | |
| /> | |
| ))} | |
| </div> | |
| <div className="flex flex-col gap-4"> | |
| <h3 className="font-semibold text-lg">Tools</h3> | |
| {toolLinks.map((link) => ( | |
| <NavLink | |
| key={link.name} | |
| name={link.name} | |
| href={link.href} | |
| onClick={() => setShowBurgerMenu(false)} | |
| /> | |
| ))} | |
| </div> | |
| <div className="flex flex-col gap-4"> | |
| <h3 className="font-semibold text-lg">APIs</h3> | |
| {apisLinks.map((link) => ( | |
| <NavLink | |
| key={link.name} | |
| name={link.name} | |
| href={link.href} | |
| onClick={() => setShowBurgerMenu(false)} | |
| /> | |
| ))} | |
| </div> | |
| <div className="flex flex-col gap-4"> | |
| <h3 className="font-semibold text-lg">Support</h3> | |
| {supportLinks.map((link) => ( | |
| <NavLink | |
| key={link.name} | |
| name={link.name} | |
| href={link.href} | |
| onClick={() => setShowBurgerMenu(false)} | |
| /> | |
| ))} | |
| </div> | |
| <NavLink | |
| name="Changelog" | |
| href="/changelog" | |
| onClick={() => setShowBurgerMenu(false)} | |
| /> | |
| </div> | |
| - </div> | |
| + </div> | |
| )} |
🤖 Prompt for AI Agents
In apps/portal/src/app/Header.tsx between lines 276 and 347, the mobile menu
overlay lacks accessibility features like focus trapping and aria-modal
attribute, and it does not lock body scroll when open. To fix this, wrap the
overlay content in a component that manages focus trapping and sets
aria-modal="true", such as the existing Dialog component used elsewhere in the
app. Additionally, implement body scroll locking while the menu is open to
prevent background scrolling and context loss. This will improve keyboard
navigation and user experience for screen reader users.
size-limit report 📦
|
joaquim-verges
force-pushed
the
_Portal_Redesign_header_with_improved_mobile_menu_layout
branch
from
32d463f to
6b73f43
Compare
joaquim-verges
force-pushed
the
_Portal_Redesign_header_with_improved_mobile_menu_layout
branch
from
6b73f43 to
70c896e
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
apps/portal/src/app/Header.tsx (1)
315-387: Mobile menu accessibility issues remain unaddressed.The mobile menu overlay still lacks the accessibility improvements mentioned in previous reviews, including focus trapping,
aria-modal="true", and scroll lock functionality. These are important for keyboard navigation and screen reader users.As suggested in the previous review, consider using the existing
Dialogcomponent which already handles these accessibility concerns, or implement the minimal improvements withrole="dialog",aria-modal="true", and scroll lock handling.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/portal/src/app/Header.tsx(5 hunks)apps/portal/src/components/others/DocSearch.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/portal/src/components/others/DocSearch.tsx
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: E2E Tests (pnpm, esbuild)
- GitHub Check: E2E Tests (pnpm, webpack)
- GitHub Check: Size
- GitHub Check: E2E Tests (pnpm, vite)
- GitHub Check: Lint Packages
- GitHub Check: Analyze (javascript)
🔇 Additional comments (2)
apps/portal/src/app/Header.tsx (2)
196-312: Well-implemented responsive header layout.The two-row header design effectively separates navigation concerns and provides good responsive behavior. The conditional rendering based on screen size breakpoints and the logical grouping of navigation elements enhance both usability and maintainability.
29-33: Good addition of Connect link with consistent structure.The new "Connect" link follows the established pattern with proper icon integration and maintains consistency with the existing navigation structure.
| const sdkLinks = [ | ||
| { | ||
| name: "TypeScript", | ||
| href: "/typescript/v5", | ||
| icon: TypeScriptIcon, | ||
| }, | ||
| { | ||
| name: "React", | ||
| href: "/react/v5", | ||
| icon: ReactIcon, | ||
| }, | ||
| { | ||
| name: "React Native", | ||
| href: "/react-native/v5", | ||
| icon: ReactIcon, | ||
| }, | ||
| { | ||
| name: ".NET", | ||
| href: "/dotnet", | ||
| icon: DotNetIcon, | ||
| }, | ||
| { | ||
| name: "Unity", | ||
| href: "/unity", | ||
| icon: UnityIcon, | ||
| }, | ||
| { | ||
| name: "Unreal Engine", | ||
| href: "/unreal-engine", | ||
| icon: UnrealEngineIcon, | ||
| }, | ||
| ]; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Eliminate code duplication between sdkLinks and connectLinks.
The newly added sdkLinks array duplicates most entries from the existing connectLinks array (lines 87-127), differing only by the missing "Overview" entry. This violates the DRY principle and creates maintenance overhead.
Consider deriving sdkLinks from connectLinks to eliminate duplication:
-const sdkLinks = [
- {
- name: "TypeScript",
- href: "/typescript/v5",
- icon: TypeScriptIcon,
- },
- {
- name: "React",
- href: "/react/v5",
- icon: ReactIcon,
- },
- {
- name: "React Native",
- href: "/react-native/v5",
- icon: ReactIcon,
- },
- {
- name: ".NET",
- href: "/dotnet",
- icon: DotNetIcon,
- },
- {
- name: "Unity",
- href: "/unity",
- icon: UnityIcon,
- },
- {
- name: "Unreal Engine",
- href: "/unreal-engine",
- icon: UnrealEngineIcon,
- },
-];
+const sdkLinks = connectLinks.filter(link => link.name !== "Overview");📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const sdkLinks = [ | |
| { | |
| name: "TypeScript", | |
| href: "/typescript/v5", | |
| icon: TypeScriptIcon, | |
| }, | |
| { | |
| name: "React", | |
| href: "/react/v5", | |
| icon: ReactIcon, | |
| }, | |
| { | |
| name: "React Native", | |
| href: "/react-native/v5", | |
| icon: ReactIcon, | |
| }, | |
| { | |
| name: ".NET", | |
| href: "/dotnet", | |
| icon: DotNetIcon, | |
| }, | |
| { | |
| name: "Unity", | |
| href: "/unity", | |
| icon: UnityIcon, | |
| }, | |
| { | |
| name: "Unreal Engine", | |
| href: "/unreal-engine", | |
| icon: UnrealEngineIcon, | |
| }, | |
| ]; | |
| -const sdkLinks = [ | |
| - { | |
| - name: "TypeScript", | |
| - href: "/typescript/v5", | |
| - icon: TypeScriptIcon, | |
| - }, | |
| - { | |
| - name: "React", | |
| - href: "/react/v5", | |
| - icon: ReactIcon, | |
| - }, | |
| - { | |
| - name: "React Native", | |
| - href: "/react-native/v5", | |
| - icon: ReactIcon, | |
| - }, | |
| - { | |
| - name: ".NET", | |
| - href: "/dotnet", | |
| - icon: DotNetIcon, | |
| - }, | |
| - { | |
| - name: "Unity", | |
| - href: "/unity", | |
| - icon: UnityIcon, | |
| - }, | |
| - { | |
| - name: "Unreal Engine", | |
| - href: "/unreal-engine", | |
| - icon: UnrealEngineIcon, | |
| - }, | |
| -]; | |
| +const sdkLinks = connectLinks.filter(link => link.name !== "Overview"); |
🤖 Prompt for AI Agents
In apps/portal/src/app/Header.tsx between lines 87-127 and 148-179, the sdkLinks
array duplicates most entries from the connectLinks array except for the
"Overview" entry. To fix this, refactor sdkLinks to be derived from connectLinks
by filtering out the "Overview" entry instead of redefining the entire array.
This will eliminate duplication and reduce maintenance overhead.
70c896e to
41971d7
Compare
joaquim-verges
deleted the
_Portal_Redesign_header_with_improved_mobile_menu_layout
branch
PR-Codex overview
This PR focuses on enhancing the layout and structure of the
Headercomponent and adding new links for SDKs in theHeaderandDocSearchcomponents.Detailed summary
ButtoninDocSearchfromw-56tow-64.isNewSDKfunction inDocSearch.sdkLinksarray with links for various SDKs.Headerlayout to improve responsiveness and organization.Summary by CodeRabbit
Refactor
Style
Chores