Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
268 changes: 218 additions & 50 deletions apps/website/src/__tests__/pages/__snapshots__/about.test.tsx.snap

Large diffs are not rendered by default.

268 changes: 218 additions & 50 deletions apps/website/src/__tests__/pages/__snapshots__/join-us.test.tsx.snap

Large diffs are not rendered by default.

6 changes: 4 additions & 2 deletions apps/website/src/components/Nav/Nav.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@ export const Nav: React.FC<NavProps> = ({ variant: variantProp }) => {
const isOnColoredBackground = variant === 'transparent';

const [expandedSections, setExpandedSections] = useState<ExpandedSectionsState>({
about: false,
courses: false,
projects: false,
explore: false,
mobileNav: false,
profile: false,
Expand All @@ -40,7 +41,8 @@ export const Nav: React.FC<NavProps> = ({ variant: variantProp }) => {

const handleBreakpointChange = () => {
setExpandedSections({
about: false,
courses: false,
projects: false,
explore: false,
mobileNav: false,
profile: false,
Expand Down
8 changes: 6 additions & 2 deletions apps/website/src/components/Nav/_MobileNavLinks.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@ export const MobileNavLinks: React.FC<{
const onToggleMobileNav = () => {
updateExpandedSections({
mobileNav: !expandedSections.mobileNav,
courses: false,
projects: false,
explore: false,
profile: false,
});
Expand All @@ -68,7 +70,8 @@ export const MobileNavLinks: React.FC<{
// Close mobile nav when any link is clicked
if ((e.target as HTMLElement).tagName === 'A') {
updateExpandedSections({
about: false,
courses: false,
projects: false,
explore: false,
mobileNav: false,
profile: false,
Expand All @@ -79,7 +82,8 @@ export const MobileNavLinks: React.FC<{
// Also handle keyboard navigation
if (e.key === 'Enter' && (e.target as HTMLElement).tagName === 'A') {
updateExpandedSections({
about: false,
courses: false,
projects: false,
explore: false,
mobileNav: false,
profile: false,
Expand Down
106 changes: 63 additions & 43 deletions apps/website/src/components/Nav/_NavLinks.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -36,14 +36,29 @@ export const NavLinks: React.FC<{
const { courses, loading } = useCourses();
const { getPrimaryCourseURL } = usePrimaryCourseURL();

const navCourses = loading ? [] : [
...(courses || []).map((course) => ({
title: course.title,
url: getPrimaryCourseURL(course.slug),
isNew: course.isNew ?? false,
})),
const allCourses = loading ? [] : (courses || []).map((course) => ({
title: course.title,
url: getPrimaryCourseURL(course.slug),
isNew: course.isNew ?? false,
type: course.type ?? null,
}));

const navCourses = [
...allCourses.filter((course) => course.type !== 'Project'),
{ title: 'See upcoming rounds', url: ROUTES.courses.url },
];

const navProjects = [
...allCourses.filter((course) => course.type === 'Project'),
{ title: 'See upcoming rounds', url: `${ROUTES.courses.url}#projects` },
];

const exploreLinks = [
{ title: 'Events', url: 'https://lu.ma/bluedotevents?utm_source=website&utm_campaign=nav', external: true },
{ title: 'Blog', url: ROUTES.blog.url, external: true },
{ title: 'Grants', url: ROUTES.grants.url },
];

const getLinkClasses = (isCurrentPathValue?: boolean) => {
// Mobile drawer always has white background, so always use dark text
// Desktop navbar uses white text on colored background, dark text elsewhere
Expand All @@ -63,49 +78,57 @@ export const NavLinks: React.FC<{
<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"
/>
Comment on lines 78 to +125

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 No tests for new grouped dropdown behaviour

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!

<A
href={ROUTES.about.url}
className={getLinkClasses(isCurrentPath(ROUTES.about.url))}
>
About
</A>
<A
href={ROUTES.grants.url}
className={getLinkClasses(isCurrentPath(ROUTES.grants.url))}
>
Grants
</A>
<A
href={ROUTES.joinUs.url}
className={getLinkClasses(isCurrentPath(ROUTES.joinUs.url))}
Expand All @@ -121,13 +144,13 @@ const NavDropdown: React.FC<{
expandedSections: ExpandedSectionsState;
isExpanded: boolean;
onColoredBackground: boolean;
links: { title: string; url: string; isNew?: boolean | null }[];
links: { title: string; url: string; isNew?: boolean | null; external?: boolean }[];
onToggle: () => void;
onClose: () => void;
title: string;
// Optional
className?: string;
loading: boolean;
loading?: boolean;
}> = ({
expandedSections,
isExpanded,
Expand All @@ -137,7 +160,7 @@ const NavDropdown: React.FC<{
onClose,
title,
className,
loading,
loading = false,
}) => {
const dropdownRef = useClickOutside(
onClose,
Expand Down Expand Up @@ -216,12 +239,13 @@ const NavDropdown: React.FC<{

return (
<React.Fragment key={link.url}>
{/* Add separator before "See upcoming rounds" */}
{link.title === 'See upcoming rounds' && (
{/* Add separator before footer links */}
{(link.title === 'See upcoming rounds' || link.title === 'See all projects') && (
Comment thread
anglilian marked this conversation as resolved.
Outdated
<div className="border-t border-gray-200 my-2" />
)}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
<A
href={link.url}
{...(link.external ? { target: '_blank', rel: 'noopener noreferrer' } : {})}
className={clsx(
'nav-link nav-link-animation w-fit no-underline text-size-sm font-[450] leading-[160%] align-middle',
'pt-1',
Expand All @@ -247,10 +271,6 @@ const NavDropdown: React.FC<{
{link.title === 'AGI Strategy' && (
<div className="border-t border-gray-200 my-2" />
)}
{/* Add divider after Technical AI Safety */}
{link.title === 'Technical AI Safety' && (
<div className="border-t border-gray-200 my-2" />
)}
</React.Fragment>
);
})
Expand Down
3 changes: 2 additions & 1 deletion apps/website/src/components/Nav/_ProfileLinks.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,8 @@ export const ProfileLinks: React.FC<{
);

const onToggleProfile = () => updateExpandedSections({
about: false,
courses: false,
projects: false,
explore: false,
mobileNav: false,
profile: !expandedSections.profile,
Expand Down
Loading
Loading