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 react-router-dom navlinks component #16

Merged
merged 1 commit into from
Apr 6, 2023
Merged

Conversation

alucernoni
Copy link
Collaborator

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

Description

Replaced tags with react-router-dom NavLink component and routed links to matching router paths

Related Issue

closes #2

Acceptance Criteria

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

Type of Changes

Type
🐛 Bug fix
✨ New feature
🔨 Refactoring
💯 Add tests
🔗 Update dependencies
📜 Docs

Updates

Before

After

Testing Steps / QA Criteria

@alucernoni alucernoni self-assigned this Apr 3, 2023
@github-actions
Copy link

github-actions bot commented Apr 3, 2023

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

https://tcl-56-smart-shopping-li-ffe7d--pr16-dn-al-navlinks-0db6pmvv.web.app

(expires Mon, 10 Apr 2023 23:07:49 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: 9c51cec5927ae3f1253a2134be95c1a07393f9a7

Copy link
Collaborator

@yiremorlans yiremorlans left a comment

Choose a reason for hiding this comment

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

simple and readable code -- great!

Copy link
Collaborator

@ashleyvalentine ashleyvalentine left a comment

Choose a reason for hiding this comment

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

Looks great!

@adidalal
Copy link
Collaborator

adidalal commented Apr 5, 2023

Not a blocker or necessarily in scope for this week, and we can discover this together - consider how we could test this?

@adidalal adidalal merged commit 1ac8238 into main Apr 6, 2023
3 checks passed
@adidalal adidalal deleted the dn-al-navlinks branch April 6, 2023 00:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants