Mm feature/navbar layout #40
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
… desktop and mobile/tablet
…bile devices, add icons to navbar to match the design
…ition for light to dark mode switch, remove unnecessary button from homepage.
… and apply custom colors to the project
…cons - Moved the Navbar component into its own dedicated file for better separation of concerns and maintainability - Added basic tooltips to non-navbar icons (dark mode toggle, sign-in/sign-out buttons)
…ttom-right of the page where it was before
joriordan332
approved these changes
Oct 5, 2024
Hudamabkhoot
requested changes
Oct 5, 2024
There was a problem hiding this comment.
Well done @marshjaja it looks amazing! 🙌 I only have a few comments
- I believe the logo is a bit big for desktop and too big for mobile.
- The nav bar active and hover state are the same color so if i'm on home and i hover over list they both look active.
- The mobile nav bar doesn't have a background so if the list is longer it wont be that visible.
|
@Hudamabkhoot good spots, thank you!👏🏾 I have implemented your requested changes. Fingers crossed and and may the power be with us when we go and do our merges🥲😅 |
Hudamabkhoot
approved these changes
Oct 6, 2024
|
Visit the preview URL for this PR (updated for commit 90e286b): https://tcl-79-smart-shopping-list--pr40-mm-feature-navbar-la-kqscvtoi.web.app (expires Sun, 13 Oct 2024 10:51:16 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: d91d9ddbda780208241c52942f544acf8e81407a |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PR introduces a fully responsive navigation system for both desktop and tablet/mobile devices.
An active link class has been applied to highlight the current page the user is on, along with hover states for the links and icons.
Additionally, a light and dark mode toggle has been implemented, allowing users to switch between these two themes.
This enhances the user experience by ensuring the navigation is intuitive across devices and provides flexibility in user interface preferences.
Related Issue
Closes #31
Acceptance Criteria
Type of Changes
featureenhancementUpdates
Before
After
Favicon
Desktop Light mode
Desktop Dark mode
Mobile Light mode
Mobile Dark mode
Testing Steps / QA Criteria
git pullandgit checkout mm-feature/navbar-layout