-
Notifications
You must be signed in to change notification settings - Fork 13
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
Navbar addition- Sylvester and Eddy #69
base: main
Are you sure you want to change the base?
Conversation
The Navbar code(jsx,module.css) and App (jsx) update for the navbar function. Navbar(s) should be in a folder in Pages andApp.jsx needs a update for functionality
Add files via upload
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
![Screenshot 2024-02-26 at 4 27 27 PM](https://private-user-images.githubusercontent.com/78942001/307994793-4348f4c5-47f3-401a-acee-e61682f6de9b.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3MjQ1MzI4MjAsIm5iZiI6MTcyNDUzMjUyMCwicGF0aCI6Ii83ODk0MjAwMS8zMDc5OTQ3OTMtNDM0OGY0YzUtNDdmMy00MDFhLWFjZWUtZTYxNjgyZjZkZTliLnBuZz9YLUFtei1BbGdvcml0aG09QVdTNC1ITUFDLVNIQTI1NiZYLUFtei1DcmVkZW50aWFsPUFLSUFWQ09EWUxTQTUzUFFLNFpBJTJGMjAyNDA4MjQlMkZ1cy1lYXN0LTElMkZzMyUyRmF3czRfcmVxdWVzdCZYLUFtei1EYXRlPTIwMjQwODI0VDIwNDg0MFomWC1BbXotRXhwaXJlcz0zMDAmWC1BbXotU2lnbmF0dXJlPTMzNmJiNDQzODRlNDc0YjNhYjM1MTE2MWUxZDBjNmYxOTg2YmI4N2RkNjNjNTg0ODM4YWE2NjhlOTdiMmY5ODMmWC1BbXotU2lnbmVkSGVhZGVycz1ob3N0JmFjdG9yX2lkPTAma2V5X2lkPTAmcmVwb19pZD0wIn0.Woi_DyQ2LUgSVvY5FSuddf1zOLXegKAb7bZyAUrhIkE)
These are the errors that the linter is complaining about. These should be fairly simple fixes.
![Screenshot 2024-02-26 at 4 47 42 PM](https://private-user-images.githubusercontent.com/78942001/307997815-76ab4dcd-0c80-494f-ab6d-1f5c66a73ce7.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3MjQ1MzI4MjAsIm5iZiI6MTcyNDUzMjUyMCwicGF0aCI6Ii83ODk0MjAwMS8zMDc5OTc4MTUtNzZhYjRkY2QtMGM4MC00OTRmLWFiNmQtMWY1YzY2YTczY2U3LnBuZz9YLUFtei1BbGdvcml0aG09QVdTNC1ITUFDLVNIQTI1NiZYLUFtei1DcmVkZW50aWFsPUFLSUFWQ09EWUxTQTUzUFFLNFpBJTJGMjAyNDA4MjQlMkZ1cy1lYXN0LTElMkZzMyUyRmF3czRfcmVxdWVzdCZYLUFtei1EYXRlPTIwMjQwODI0VDIwNDg0MFomWC1BbXotRXhwaXJlcz0zMDAmWC1BbXotU2lnbmF0dXJlPWUyZjFmOGYyODhhYjMxMjM0YzU1NjkwMjllNTNiYzY3ZmRmNzcwODE3NTg4NTlmYzg2YTMwMWM0ODY0M2NlNjMmWC1BbXotU2lnbmVkSGVhZGVycz1ob3N0JmFjdG9yX2lkPTAma2V5X2lkPTAmcmVwb19pZD0wIn0.GLLQONOyc_8Scso3GkKVUsUWr3U-Hcg8Kai3ad75DsA)
You can view this information by clicking on Details
.
Some changes I would suggest would be changing px to either em or rem in the css file for Navbar to make it more responsive. Also, the Navbar folder and its files should be under the Components folder, which is under frontend/src
. And just wondering, are you editing the already existing App.jsx file? From this pull request, it looks like you created a new folder with the App files copied over instead of directly editing the App files under frontend/src/Pages/App
. Can you copy over those changes to frontend/src/Pages/App/App.jsx
instead? And move the Navbar folder into frontend/src/Components
.
Lastly, can you include a image of the navbar so others reviewing the pull request can easily see what you did?
Add Navbar Test
@@ -0,0 +1,4 @@ | |||
.app-page{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great, awesome work! Only change requested is to move this css file under the App folder in the frontend folder. And delete the extra App Folder so that this PR only has the Frontend/app folder.
These are the files for the Navbar. The changes made are the new Navbar folder in Pages. They are Navbar.jsx, Navbar.module.css, App.jsx and