www
/
www-new
12
2
Fork 2
Code Issues 58 Pull Requests 30 Projects Releases Wiki Activity
Labels Milestones

Fix navbar highlight for Organized Content sections #182

Merged
a258wang merged 10 commits from amy-navbar-highlight-fix into main 1 year ago
Conversation 2 Commits 10 Files Changed 9
a258wang commented 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
continuous-integration/drone/push Build is passing
Details
db9f5f1c5c Fix navbar highlight for pages with Organized Content
a3thakra reviewed 1 year ago
components/Navbar.tsx Outdated
const isCurrentPage =
router.pathname === props.route ||
(props.submenu != null &&
router.pathname.startsWith(getMainRoute(props.route)));
a3thakra commented 1 year ago
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))); ```
a258wang commented 1 year ago
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
a258wang commented 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`. :)
👍 1
a3thakra reviewed 1 year ago
components/Navbar.tsx Outdated
submenu?: {
name: string;
route: string;
hasSubsections?: boolean;
a3thakra commented 1 year ago
Collaborator

We don't need this anymore, correct?

We don't need this anymore, correct?
a258wang commented 1 year ago
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
continuous-integration/drone/push Build is passing
Details
bf7a07ca7f Merge branch 'main' into amy-navbar-highlight-fix
a3thakra reviewed 1 year ago
components/Navbar.tsx Outdated
ancestors: { name: string; route: string }[];
}[];
function dfs(
a3thakra commented 1 year ago
Collaborator

I think this function should be called something like collectLeaves

I think this function should be called something like `collectLeaves`
a3thakra commented 1 year ago
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
components/Navbar.tsx Outdated
}
let ret = [] as Leaves;
for (const subEntry of entry.submenu) {
ret = ret.concat(dfs(subEntry, [...ancestors, entry]));
a3thakra commented 1 year ago
Collaborator

You can use push

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

You can use `push` `ret.push(...dfs(subEntry, [...ancestors, entry]));`
a3thakra commented 1 year ago
Collaborator

Just some nitpicks, looks good otherwise! :)

Just some nitpicks, looks good otherwise! :)
a258wang added 2 commits 1 year ago
continuous-integration/drone/push Build is passing
Details
4204d2ea93 Clean up collectLeaves
a258wang added 1 commit 1 year ago
continuous-integration/drone/push Build is passing
Details
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
Details
The pull request has been merged as 0bb8049db3.
Sign in to join this conversation.
Reviewers
Request review
No reviewers
a3thakra
Labels
Apply labels
Clear labels
a11y Backlog Blocked Bug Content Dependencies Design Feature Request Good First Issue In Progress Performance Priority - High Priority - Low Priority - Medium Untriaged
No Label a11y Backlog Blocked Bug Content Dependencies Design Feature Request Good First Issue In Progress Performance Priority - High Priority - Low Priority - Medium Untriaged
Milestone
Set milestone
Clear milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Assign users
Clear assignees
No Assignees
2 Participants
Notifications
Due Date

No due date set.

Dependencies

No dependencies set.

Reference: www/www-new#182
Write Preview
Loading…
Cancel
Save
There is no content yet.
Delete Branch 'amy-navbar-highlight-fix'

Deleting a branch is permanent. It CANNOT be undone. Continue?

No
Yes