www/www-new
16
3
Fork 11
Code Issues 60 Pull Requests 47 Projects Releases Wiki Activity

Fix navbar highlight for Organized Content sections #182

Merged
a258wang merged 10 commits from amy-navbar-highlight-fix into main 2021-08-31 23:07:21 -04:00
Conversation 7 Commits 10 Files Changed 9 +96 -19
a258wang commented 2021-08-26 16:26:20 -04:00
Owner
Copy Link

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 2021-08-26 16:26:21 -04:00
Fix navbar highlight for pages with Organized Content
All checks were successful
continuous-integration/drone/push Build is passing
Details
db9f5f1c5c
a3thakra reviewed 2021-08-27 14:49:31 -04:00
components/Navbar.tsx Outdated
@ -201,7 +211,8 @@ function NavItem(props: NavItemProps) {
const isCurrentPage =
router.pathname === props.route ||
(props.submenu != null &&
router.pathname.startsWith(getMainRoute(props.route)));
a3thakra commented 2021-08-27 14:49:31 -04:00
Owner
Copy Link

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 2021-08-27 16:05:45 -04:00
Author
Owner
Copy Link

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 2021-08-28 18:22:28 -04:00
a258wang commented 2021-08-28 18:24:23 -04:00
Author
Owner
Copy Link

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 2021-08-28 22:02:22 -04:00
components/Navbar.tsx Outdated
@ -190,3 +205,4 @@
submenu?: {
name: string;
route: string;
hasSubsections?: boolean;
a3thakra commented 2021-08-28 22:02:22 -04:00
Owner
Copy Link

We don't need this anymore, correct?

We don't need this anymore, correct?
a258wang commented 2021-08-28 22:49:24 -04:00
Author
Owner
Copy Link

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 2021-08-28 22:56:08 -04:00
Merge branch 'main' into amy-navbar-highlight-fix
All checks were successful
continuous-integration/drone/push Build is passing
Details
bf7a07ca7f
a3thakra reviewed 2021-08-29 21:33:30 -04:00
components/Navbar.tsx Outdated
@ -276,0 +295,4 @@
ancestors: { name: string; route: string }[];
}[];
function dfs(
a3thakra commented 2021-08-29 21:33:30 -04:00
Owner
Copy Link

I think this function should be called something like collectLeaves

I think this function should be called something like `collectLeaves`
a3thakra commented 2021-08-29 21:36:55 -04:00
Owner
Copy Link

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 2021-08-29 21:40:21 -04:00
components/Navbar.tsx Outdated
@ -276,0 +304,4 @@
}
let ret = [] as Leaves;
for (const subEntry of entry.submenu) {
ret = ret.concat(dfs(subEntry, [...ancestors, entry]));
a3thakra commented 2021-08-29 21:40:21 -04:00
Owner
Copy Link

You can use push

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

You can use `push` `ret.push(...dfs(subEntry, [...ancestors, entry]));`
a3thakra commented 2021-08-29 21:43:16 -04:00
Owner
Copy Link

Just some nitpicks, looks good otherwise! :)

Just some nitpicks, looks good otherwise! :)
a258wang added 2 commits 2021-08-30 23:28:43 -04:00
Clean up collectLeaves
All checks were successful
continuous-integration/drone/push Build is passing
Details
4204d2ea93
a258wang added 1 commit 2021-08-30 23:30:26 -04:00
Merge branch 'main' into amy-navbar-highlight-fix
All checks were successful
continuous-integration/drone/push Build is passing
Details
8f5418c312
a3thakra approved these changes 2021-08-31 21:53:10 -04:00
a258wang merged commit 0bb8049db3 into main 2021-08-31 23:07:21 -04:00
Sign in to join this conversation.
Reviewers
No Reviewers
a3thakra
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
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
No Assignees
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: www/www-new#182
No description provided.
Delete Branch "amy-navbar-highlight-fix"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?