Fix navbar highlight for Organized Content sections #182

Merged
a258wang merged 10 commits from amy-navbar-highlight-fix into main 1 year ago
Owner

Also fixes the navbar highlight for the different Advice sections.

Closes #181

Also fixes the navbar highlight for the different Advice sections. Closes #181
a258wang added 1 commit 1 year ago
db9f5f1c5c Fix navbar highlight for pages with Organized Content
a3thakra reviewed 1 year ago
const isCurrentPage =
router.pathname === props.route ||
(props.submenu != null &&
router.pathname.startsWith(getMainRoute(props.route)));
Collaborator

What if we do:

const isCurrentPage =
    router.pathname === props.route ||
    (router.pathname !== "/" &&
      router.pathname.startsWith(getMainRoute(props.route)));
What if we do: ```ts const isCurrentPage = router.pathname === props.route || (router.pathname !== "/" && router.pathname.startsWith(getMainRoute(props.route))); ```
Poster
Owner

That doesn't work because whenever we navigate to anything in the About submenu, all of the other entries in the submenu will also be highlighted blue. (Same for the Resources submenu.)

That doesn't work because whenever we navigate to anything in the About submenu, all of the other entries in the submenu will also be highlighted blue. (Same for the Resources submenu.)
a3thakra marked this conversation as resolved
a258wang added 4 commits 1 year ago
Poster
Owner

Side note: I ended up keeping the About Us page at /about because I was able to make it work, but let me know if we'd prefer to move it to /about/us. :)

Side note: I ended up keeping the About Us page at `/about` because I was able to make it work, but let me know if we'd prefer to move it to `/about/us`. :)
a3thakra reviewed 1 year ago
submenu?: {
name: string;
route: string;
hasSubsections?: boolean;
Collaborator

We don't need this anymore, correct?

We don't need this anymore, correct?
Poster
Owner

Oops yes sorry, I'll get rid of it! I had to do some mergey stuff and I must've forgotten to delete this line lol

Oops yes sorry, I'll get rid of it! I had to do some mergey stuff and I must've forgotten to delete this line lol
a3thakra marked this conversation as resolved
a258wang added 2 commits 1 year ago
a3thakra reviewed 1 year ago
ancestors: { name: string; route: string }[];
}[];
function dfs(
Collaborator

I think this function should be called something like collectLeaves

I think this function should be called something like `collectLeaves`
Collaborator

Let's make the signature of this function similar to what reduce takes in. Then we could do

const leaves = menu.reduce(collectLeaves, [] as Leaves)

(or something like this)

Let's make the signature of this function similar to what reduce takes in. Then we could do ```ts const leaves = menu.reduce(collectLeaves, [] as Leaves) ``` (or something like this)
a3thakra reviewed 1 year ago
}
let ret = [] as Leaves;
for (const subEntry of entry.submenu) {
ret = ret.concat(dfs(subEntry, [...ancestors, entry]));
Collaborator

You can use push

ret.push(...dfs(subEntry, [...ancestors, entry]));

You can use `push` `ret.push(...dfs(subEntry, [...ancestors, entry]));`
Collaborator

Just some nitpicks, looks good otherwise! :)

Just some nitpicks, looks good otherwise! :)
a258wang added 2 commits 1 year ago
a258wang added 1 commit 1 year ago
8f5418c312 Merge branch 'main' into amy-navbar-highlight-fix
a3thakra approved these changes 1 year ago
a258wang merged commit 0bb8049db3 into main 1 year ago

Reviewers

a3thakra approved these changes 1 year ago
continuous-integration/drone/push Build is passing
The pull request has been merged as 0bb8049db3.
Sign in to join this conversation.
No reviewers
No Milestone
No project
No Assignees
2 Participants
Notifications
Due Date

No due date set.

Dependencies

No dependencies set.

Reference: www/www-new#182
Loading…
There is no content yet.