Skip to content

[CLIENT] [COMPONENT] : Navigation bar in application #12

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

Merged
merged 11 commits into from
May 25, 2025

Conversation

SUVAJIT-KARMAKAR
Copy link
Contributor

PULL REQUEST SUMMARY

This feature includes developing the navigation bar component for the application


FILES THAT ARE WORKED ON

FILES TOUCHED UPDATED ANY EXISTING CODE WRITTEN SOMETHING NEW
client/index.html yes yes
client/src/App.tsx yes yes
src/NavigationBar.tsx no yes
src/Navigation.styles.css no yes
src/Layout.tsx no yes
src/NavigationLinks.ts no yes
src/NavigationLinks.types.ts no yes
client/src/global.css yes yes
client/Logo.tsx no yes
client/Logo.types.tsx no yes
client/Loader.tsx no yes
client/Loader.styles.css no yes

ANY ISSUES WORKED ON

ISSUE ISSUE LINK
#6 PROJECT

PROVIDE EVIDANCE OF WORK

Screen.Recording.2025-05-24.at.11.12.05.PM.mov

OWNERSHIP

MESSAGE CONFIRMATION
I confirm that I am responsible for this pull request yes

ACCEPTANCE OF MY WORK

MESSAGE CONFIRMATION
I have worked on all the acceptance criteria yes

GIT FOLLOW UPS

COMMAND EFFECT
git pull main Checkout to the main branch, this will up-to-date your local with remote
git merge origin/main Merges all the up-to-date changes of your local main to local feature branch

@SUVAJIT-KARMAKAR SUVAJIT-KARMAKAR requested a review from a team May 24, 2025 18:18
@SUVAJIT-KARMAKAR SUVAJIT-KARMAKAR added client Frontend work prospective component Well its a work on a component, maybe you have to create a new or work on an existing one functional Well it's all functional but I need you to make it functional labels May 24, 2025
Copy link

@h4jack h4jack left a comment

Choose a reason for hiding this comment

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

Everything looks good overall — just a small UI detail to point out:

When we are in the root path (/), the Home tab in the navigation bar should be active. Currently, it's only active when the path is /home.

Current Behavior:

Screenshot 2025-05-25 001110

Expected Behavior:

Screenshot 2025-05-25 001136

Not a major issue, but thought I’d flag it for polish. Great work otherwise!

@SUVAJIT-KARMAKAR
Copy link
Contributor Author

Everything looks good overall — just a small UI detail to point out:

When we are in the root path (/), the Home tab in the navigation bar should be active. Currently, it's only active when the path is /home.

Current Behavior:

Screenshot 2025-05-25 001110

Expected Behavior:

Screenshot 2025-05-25 001136

Not a major issue, but thought I’d flag it for polish. Great work otherwise!

Damn brother, thats one of the best replied comments I have ever gotten ! And thanks for the attention to detail. I will fix this ASAP ! 🍾

@SUVAJIT-KARMAKAR
Copy link
Contributor Author

The required changes are done @h4jack ! Thanks for the feedback man ❤️

Copy link
Contributor

@Hemangshu-Dey Hemangshu-Dey left a comment

Choose a reason for hiding this comment

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

Just look into it .

Copy link
Contributor

@Hemangshu-Dey Hemangshu-Dey 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!

@Hemangshu-Dey Hemangshu-Dey merged commit d459564 into main May 25, 2025
@Hemangshu-Dey Hemangshu-Dey deleted the component/navigation branch May 25, 2025 14:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
client Frontend work prospective component Well its a work on a component, maybe you have to create a new or work on an existing one functional Well it's all functional but I need you to make it functional
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants