Feature/nav grouped dropdowns#2186
Conversation
…sections Refactored the navigation component to split courses into two distinct sections: - Added separate "Courses" dropdown for non-project courses - Added new "Projects" dropdown for project-type courses - Moved "Events" link into an "Explore" dropdown alongside Blog and Grants - Updated state management to handle the new navigation structure - Maintained responsive behavior across mobile and desktop views This improves navigation clarity by categorizing content more intuitively and provides better organization for different course types.
- Add dedicated projects section with visual separator and mobile heading - Update navigation to link directly to projects section via anchor (#projects) - Split course sorting logic to handle courses and projects separately - Enhance breadcrumb menu to show projects as distinct category - Maintain consistent display ordering for both courses and projects
…ropdown Cleaned up navigation dropdown by removing an unnecessary divider that was placed after the "Technical AI Safety" menu item. This simplifies the menu structure and improves visual consistency.
Refactor navigation to include dropdown menus for "Projects" and "Explore" sections, replacing direct links with expandable menu items. Updates include new dropdown components with proper ARIA attributes, responsive styling, and organized sub-navigation links for better content discovery.
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis pull request restructures the navigation component's dropdown state management by replacing the Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryThis PR refactors the website navigation bar to split the previous single "Courses" dropdown into three separate grouped dropdowns — Courses, Projects, and Explore — decluttering the top-level nav by grouping Events, Blog, and Grants under "Explore", and separating courses from projects across both desktop and mobile layouts. The courses index page is also updated to render courses and projects in distinct sections with a shared breadcrumb menu. Key changes:
Confidence Score: 3/5
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
User([User clicks nav button]) --> NavLinks
NavLinks --> CoursesBtn[Courses button]
NavLinks --> ProjectsBtn[Projects button]
NavLinks --> ExploreBtn[Explore button]
CoursesBtn --> |"updateExpandedSections\n{ courses: !courses,\n projects: false,\n explore: false }"| CoursesDropdown[Courses Dropdown]
ProjectsBtn --> |"updateExpandedSections\n{ courses: false,\n projects: !projects,\n explore: false }"| ProjectsDropdown[Projects Dropdown]
ExploreBtn --> |"updateExpandedSections\n{ courses: false,\n projects: false,\n explore: !explore }"| ExploreDropdown[Explore Dropdown]
CoursesDropdown --> |"allCourses.filter\ntype !== 'Project'"| CourseLinks[Course links\n+ 'See upcoming rounds']
ProjectsDropdown --> |"allCourses.filter\ntype === 'Project'"| ProjectLinks[Project links\n+ 'See upcoming rounds → #projects']
ExploreDropdown --> StaticLinks[Events / Blog / Grants]
CourseLinks --> |"useClickOutside"| CloseAll[Close dropdown]
ProjectLinks --> |"useClickOutside"| CloseAll
ExploreDropdown --> |"useClickOutside"| CloseAll
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| // Sort courses by fixed display order | ||
| const sortedCourses = useMemo(() => { | ||
| return [...displayedCourses].sort((a, b) => { | ||
| const sortByDisplayOrder = (items: Course[]) => { |
There was a problem hiding this comment.
sortByDisplayOrder missing from useMemo dependencies
sortByDisplayOrder is defined inside useSortedCourses but is not included in the dependency arrays of sortedCourses or sortedProjects. This will trigger the react-hooks/exhaustive-deps lint rule. Since sortByDisplayOrder doesn't capture any reactive values, moving it outside the hook (or wrapping it in useCallback) is the cleanest fix.
| const sortByDisplayOrder = (items: Course[]) => { | |
| const sortByDisplayOrder = useCallback((items: Course[]) => { |
You'll also need to add useCallback to the import at the top and add sortByDisplayOrder to the useMemo dependency arrays.
Rule Used: Consider refactoring complex lines of code for bet... (source)
Learnt From
bluedotimpact/bluedot#963
| <div className={clsx('nav-links flex gap-9 [&>*]:w-fit', className)}> | ||
| <NavDropdown | ||
| expandedSections={expandedSections} | ||
| isExpanded={expandedSections.explore} | ||
| isExpanded={expandedSections.courses} | ||
| onColoredBackground={onColoredBackground} | ||
| links={navCourses} | ||
| onToggle={() => updateExpandedSections({ | ||
| about: false, | ||
| explore: !expandedSections.explore, | ||
| courses: !expandedSections.courses, | ||
| projects: false, | ||
| explore: false, | ||
| mobileNav: expandedSections.mobileNav, | ||
| profile: false, | ||
| })} | ||
| onClose={() => updateExpandedSections({ explore: false })} | ||
| onClose={() => updateExpandedSections({ courses: false })} | ||
| title="Courses" | ||
| loading={loading} | ||
| /> | ||
| <A | ||
| href="https://lu.ma/bluedotevents?utm_source=website&utm_campaign=nav" | ||
| target="_blank" | ||
| rel="noopener noreferrer" | ||
| className={getLinkClasses()} | ||
| aria-label="Events (opens in new tab)" | ||
| > | ||
| Events | ||
| </A> | ||
| <A | ||
| href={ROUTES.blog.url} | ||
| target="_blank" | ||
| rel="noopener noreferrer" | ||
| className={getLinkClasses(isCurrentPath(ROUTES.blog.url))} | ||
| aria-label="Blog (opens in new tab)" | ||
| > | ||
| Blog | ||
| </A> | ||
| <NavDropdown | ||
| expandedSections={expandedSections} | ||
| isExpanded={expandedSections.projects} | ||
| onColoredBackground={onColoredBackground} | ||
| links={navProjects} | ||
| onToggle={() => updateExpandedSections({ | ||
| courses: false, | ||
| projects: !expandedSections.projects, | ||
| explore: false, | ||
| mobileNav: expandedSections.mobileNav, | ||
| profile: false, | ||
| })} | ||
| onClose={() => updateExpandedSections({ projects: false })} | ||
| title="Projects" | ||
| loading={loading} | ||
| /> | ||
| <NavDropdown | ||
| expandedSections={expandedSections} | ||
| isExpanded={expandedSections.explore} | ||
| onColoredBackground={onColoredBackground} | ||
| links={exploreLinks} | ||
| onToggle={() => updateExpandedSections({ | ||
| courses: false, | ||
| projects: false, | ||
| explore: !expandedSections.explore, | ||
| mobileNav: expandedSections.mobileNav, | ||
| profile: false, | ||
| })} | ||
| onClose={() => updateExpandedSections({ explore: false })} | ||
| title="Explore" | ||
| /> |
There was a problem hiding this comment.
The refactoring introduces three separate NavDropdown instances (Courses, Projects, Explore) with mutual-exclusion logic (opening one closes the others). This interactive behaviour is a good candidate for functional tests — e.g. verifying that clicking "Courses" closes the "Projects" dropdown, or that keyboard navigation works correctly. Consider adding tests alongside the snapshot updates.
Rule Used: Consider adding tests for any new functionality in... (source)
Learnt From
bluedotimpact/bluedot#956
bluedotimpact/bluedot#969
bluedotimpact/bluedot#958
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
![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
🧹 Nitpick comments (1)
apps/website/src/pages/courses/index.tsx (1)
51-73: Consider extractingsortByDisplayOrderoutside the hook.The
sortByDisplayOrderfunction is recreated on every render since it's defined inside the hook. Although it's used withinuseMemocalls (so the sorting itself is memoized), the function recreation is a minor inefficiency. Since it only depends on the staticCOURSE_DISPLAY_ORDERconstant, it could be extracted to module scope.♻️ Optional: Extract to module scope
+/* Sort items by fixed display order */ +const sortByDisplayOrder = (items: Course[]) => { + return [...items].sort((a, b) => { + const aIndex = COURSE_DISPLAY_ORDER.indexOf(a.slug); + const bIndex = COURSE_DISPLAY_ORDER.indexOf(b.slug); + + if (aIndex !== -1 && bIndex !== -1) { + return aIndex - bIndex; + } + + if (aIndex !== -1) { + return -1; + } + + if (bIndex !== -1) { + return 1; + } + + return a.title.localeCompare(b.title); + }); +}; + /* Custom hook to fetch and sort courses with their round data */ const useSortedCourses = () => { const { data: courses, isLoading: coursesLoading, error } = trpc.courses.getAll.useQuery(); const displayedCourses = useMemo(() => { if (!courses) { return []; } return courses.filter((course) => course.displayOnCourseHubIndex); }, [courses]); - // Sort courses by fixed display order - const sortByDisplayOrder = (items: Course[]) => { - return [...items].sort((a, b) => { - const aIndex = COURSE_DISPLAY_ORDER.indexOf(a.slug); - const bIndex = COURSE_DISPLAY_ORDER.indexOf(b.slug); - - if (aIndex !== -1 && bIndex !== -1) { - return aIndex - bIndex; - } - - if (aIndex !== -1) { - return -1; - } - - if (bIndex !== -1) { - return 1; - } - - return a.title.localeCompare(b.title); - }); - }; - const sortedCourses = useMemo(() => sortByDisplayOrder(displayedCourses.filter((c) => c.type !== 'Project')), [displayedCourses]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/website/src/pages/courses/index.tsx` around lines 51 - 73, Move the sortByDisplayOrder function out of the component/module scope so it is not recreated on every render; it only depends on the constant COURSE_DISPLAY_ORDER. Replace the inline definition used by sortedCourses and sortedProjects (which call useMemo over displayedCourses) with the newly exported module-level function named sortByDisplayOrder, keeping its current logic and references to COURSE_DISPLAY_ORDER, and ensure sortedCourses and sortedProjects still call the same function inside their useMemo callbacks.
🤖 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/website/src/components/Nav/_NavLinks.tsx`:
- Around line 242-245: The condition checking for link.title === 'See all
projects' is dead because navProjects uses 'See upcoming rounds'; update the
separator condition in the NavLinks component to remove the redundant 'See all
projects' check and only test for the actual footer title (e.g., keep link.title
=== 'See upcoming rounds'), or if you intended a different footer label, make
navProjects and the condition consistent by renaming the footer link; locate the
check that references link.title and adjust accordingly.
---
Nitpick comments:
In `@apps/website/src/pages/courses/index.tsx`:
- Around line 51-73: Move the sortByDisplayOrder function out of the
component/module scope so it is not recreated on every render; it only depends
on the constant COURSE_DISPLAY_ORDER. Replace the inline definition used by
sortedCourses and sortedProjects (which call useMemo over displayedCourses) with
the newly exported module-level function named sortByDisplayOrder, keeping its
current logic and references to COURSE_DISPLAY_ORDER, and ensure sortedCourses
and sortedProjects still call the same function inside their useMemo callbacks.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 929a4074-116f-4fcd-8bcf-cbd21d6739b7
⛔ Files ignored due to path filters (6)
apps/website/src/__tests__/pages/__snapshots__/about.test.tsx.snapis excluded by!**/*.snapapps/website/src/__tests__/pages/__snapshots__/join-us.test.tsx.snapis excluded by!**/*.snapapps/website/src/components/Nav/__snapshots__/Nav.test.tsx.snapis excluded by!**/*.snapapps/website/src/components/homepage/__snapshots__/HomeHeroContent.test.tsx.snapis excluded by!**/*.snapapps/website/src/components/lander/__snapshots__/AgiStrategyLander.test.tsx.snapis excluded by!**/*.snapapps/website/src/components/lander/__snapshots__/IncubatorWeekLander.test.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (6)
apps/website/src/components/Nav/Nav.tsxapps/website/src/components/Nav/_MobileNavLinks.tsxapps/website/src/components/Nav/_NavLinks.tsxapps/website/src/components/Nav/_ProfileLinks.tsxapps/website/src/components/Nav/utils.tsapps/website/src/pages/courses/index.tsx
render
Bot
temporarily deployed
to
feature/nav-grouped-dropdowns - bluedot-preview PR #2186
1 participant
Description
Splitting apart courses and projects. Decluttering the nav bar
Issue
Fixes #
Developer checklist
Screenshot