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 NavLink to the nav element #18

Merged
merged 1 commit into from
Aug 17, 2024
Merged

Added NavLink to the nav element #18

merged 1 commit into from
Aug 17, 2024

Conversation

Wyna-7
Copy link
Collaborator

@Wyna-7 Wyna-7 commented Aug 12, 2024

Description

We made the nav bar functional. Replaced anchor tags with NavLink elements from react router dom.

Related Issue

Issue #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

Added nav bar feature.

Updates

Before

NA, no changes to UI.

After

NA, no changes to UI.

Testing Steps / QA Criteria

• Do a git pull and git checkout hm-qg-nav-routing.
• Open the homepage by running npm start.
• You will find a nav bar with 3 links at the bottom. As you click each link, the url and the hello message should change accordingly.

Copy link

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

https://tcl-79-smart-shopping-list--pr18-hm-qg-nav-routing-psxf5j00.web.app

(expires Mon, 19 Aug 2024 14:28:30 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: d91d9ddbda780208241c52942f544acf8e81407a

Copy link
Collaborator

@marshjaja marshjaja 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!

Copy link
Member

@hariscs hariscs left a comment

Choose a reason for hiding this comment

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

Great job guys 🙌

Comment on lines 9 to 10
/**
* TODO: The links defined in this file don't work!
Copy link
Member

Choose a reason for hiding this comment

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

As our links are working now, let's remove this comment

@Hudamabkhoot Hudamabkhoot merged commit 9b57689 into main Aug 17, 2024
3 checks passed
@Hudamabkhoot Hudamabkhoot deleted the hm-qg-nav-routing branch August 17, 2024 11:51
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