gevuong / minimal-i18n-with-app-router

Building the marketing site so it's deployable to GPages @virufy. It's statically exported, supports build time image optimization and internationalized routing with App Router.
https://gevuong.github.io/minimal-i18n-with-app-router/
MIT License
0 stars 5 forks source link

refactor setActiveLink functionality in Navbar #38

Closed gevuong closed 2 months ago

gevuong commented 2 months ago

At the time of writing this, in the Navbar component, there are 18 nested ternary conditions in useEffect..

As each new language gets added, theoretically, there will be 9 more nested ternary conditions under the already existing 18..

Suggested improvements:

  1. Refactor nested ternary conditions in useEffect so it's more maintainable and scalable.
  2. Create smaller reusable components (ie. Donate modal, Dropdown options)
gevuong commented 2 months ago

Please also address the following warning error in the build logs:

./app/[lang]/Navbar.tsx
31:3  Warning: React Hook useEffect contains a call to 'setActiveLink'. Without a list of dependencies, this can lead to an infinite chain of updates. To fix this, pass [lang] as a second argument to the useEffect Hook.  react-hooks/exhaustive-deps

https://github.com/gevuong/minimal-i18n-with-app-router/actions/runs/10396848527/job/28791570780#step:8:17

gevuong commented 2 months ago

@a-wong-8 are the active links working in mobile view for you? And are they supposed to work? I'm noticing the underlines appear in mobile view in the old site: https://creative-jalebi-f8e18b.netlify.app/ai. Also, the menu closes after a user selects a page to go to in the mobile navbar, but it doesn't do so in the current site, hmmm. Any ideas?

a-wong-8 commented 2 months ago

@gevuong i believe the navbar active links arent working on desktop or mobile because there is a basepath minimal-i18n-with-app-router/. It should work once it is removed.

Isnt that a desirable effect (dropdown closing)?

gevuong commented 2 months ago

Oh ok. Correct, the desirable effect is to have the dropdown close upon selecting a page. It wasn't working when I tried earlier but it seems to be working now.

a-wong-8 commented 2 months ago

@gevuong i was thinking about refactoring the navbar to use useNavigation like footer, that should correct the active link also.

gevuong commented 2 months ago

I don't see useNavigation in Footer

a-wong-8 commented 2 months ago

my bad I meant usePathname

gevuong commented 2 months ago

give it a try, lmk. Active links appear to be working in the footer on the deployed site