Skip to content
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

added route handling and added activestatus using navlink #18

Merged
merged 1 commit into from
Aug 17, 2024

Conversation

arandel1
Copy link
Collaborator

@arandel1 arandel1 commented Aug 12, 2024

For an example of how to fill this template out, see this Pull Request.

Description

Marcos and I added functionality to clicking Home, List, and Manage list links in the navbar using NavLink. We added active status feature to enhance UX, so that users can see which page they are actively viewing.

Related Issue

Issue 3: As a user, I want to be able to navigate to all the pages in the application by clicking the links in the nav bar
[#3] (#3

Acceptance Criteria

  • The nav element is added to the Layout component
  • Links to the "Home", "List", and "Add item" pages of the app are In the nav element
  • The links all function as expected using the NavLink component from react-router-dom

Type of Changes

enhancement

Updates

Screen Shot 2024-08-12 at 10 13 29 AM

Before

C08CF354-3B99-4D39-A54F-10747B2D79BD

Screen Shot 2024-08-12 at 10 14 21 AM

After

Testing Steps / QA Criteria

@arandel1
Copy link
Collaborator Author

@MarcosPerez16

Copy link

Visit the preview URL for this PR (updated for commit be4c03f):

https://tcl-76-smart-shopping-list--pr18-ar-mp-nav-to-all-3un54fzk.web.app

(expires Mon, 19 Aug 2024 15:16:26 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: 512b1a88be8ae05fd3e727b99332819df760271d

@arandel1 arandel1 requested review from EmmaBin and sar-mko August 13, 2024 14:46
@EmmaBin
Copy link
Collaborator

EmmaBin commented Aug 13, 2024

lgtm, I really like the enhanced UI for active and pending status.

Copy link
Collaborator

@sar-mko sar-mko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

<Outlet />
</main>
<nav className="Nav">
<div className="Nav-container">
<a href="#" className="Nav-link">
<NavLink to="/" style={handleActive}>
Home
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice implementation of this to handle the active route

Copy link
Collaborator

@mentalcaries mentalcaries left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work! Good clean implementation of the AC

@mentalcaries mentalcaries merged commit 02f3ffe into main Aug 17, 2024
3 checks passed
@mentalcaries mentalcaries deleted the ar-mp-nav-to-all branch August 17, 2024 22:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants