-
-
Notifications
You must be signed in to change notification settings - Fork 28
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Website: Nav bar optimization #1000
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
📝 WalkthroughWalkthroughThis pull request involves localization and UI modifications for the website. The changes include updating the French translation for the "our-promise" key in the localization file and making visual adjustments to the navbar component. The navbar's layout and styling have been refined, with specific changes to height, logo positioning, and dropdown behaviors. Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Visit the preview URL for this PR (updated for commit 3a537b5): https://si-admin-staging--pr1000-sandino-nav-wip-zn0re47a.web.app (expires Sat, 11 Jan 2025 19:28:38 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: b7b0969384059dce6ea8fad1ee1d1737e54e6676 |
…v-wip # Conflicts: # website/src/components/navbar/navbar-client.tsx
…v-wip # Conflicts: # website/src/components/navbar/navbar-client.tsx
…v-wip # Conflicts: # website/src/components/navbar/navbar-client.tsx
@mkue The issues have been resolved. As discussed, the navigation bar has become quite complex over time, and there’s certainly room for simplification. However, it functions as expected for now. I’ve tested it on an iPad as well as on a Mac using Safari, Chrome, and Firefox. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
website/src/components/navbar/navbar-client.tsx (1)
338-345
: Flag dimension adjustments.
Changing from 24px to 20px can mitigate layout issues on narrower displays. Validate if the smaller size remains recognizable.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
shared/locales/fr/website-common.json
(1 hunks)website/src/components/navbar/navbar-client.tsx
(5 hunks)website/src/components/navbar/navbar.tsx
(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- website/src/components/navbar/navbar.tsx
🔇 Additional comments (6)
website/src/components/navbar/navbar-client.tsx (5)
279-290
: Streamlined transitions and spacing look good.
Hiding the navigation by default (h-20) and then expanding on hover (h-60) is a neat approach, but make sure the transition doesn't create unexpected layout shifts on slow networks.
300-305
: Proper spacing and hover for "our-work" dropdown.
Thewhitespace-nowrap px-2
class prevents multi-line wrapping, which is ideal for keeping items tidy. Looks effective.
313-317
: Consistent approach for "about-us" dropdown.
Same pattern withwhitespace-nowrap px-2
ensures all nav items align similarly. This reduces clutter on smaller screens.
325-329
: Transparency dropdown logic matches other sections.
Uniform handling of sub-navigation fosters a cohesive user experience.
Line range hint
353-392
: Enhanced language, region, and currency selection.
Using grid for the dropdown is an excellent way to organize items. Ensure that focusing or tabbing through these items remains accessible, especially when the dropdown transitions from hidden to visible.shared/locales/fr/website-common.json (1)
11-11
: Translation text updated.
Changing"Modèle à 100 %"
to"Notre promesse"
aligns with more intuitive messaging for French speakers.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very nice!
Fixes Navi Issues:
Screenshot of issues:
Summary by CodeRabbit
Localization
User Interface
Navigation