#193 Navigation Bar #212
No reviewers
Labels
No labels
_CRITICAL_
API
app
backEnd
Blocked, waiting for further changes
bug
cleanup
close
design
duplicate
enhancement
feature request
frontEnd
help wanted
invalid
low priority
needs input
needs review
project documentation
question
research
reviewed
script
security
SQL
style
testing
topLevel
wontfix
No milestone
No project
3 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
Andras/BoundlessFlowCampus2K!212
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "#193-Navigation-Bar"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Original branch had to be deleted because i accidentally commit files that shouldn't have been.
Navbar implementation using react, cards need to be implemented using the navbar home div, since otherwise the cards will show up in the settings page and any other future page implementations.
All logos have to be replaced with our own at some point, current logos are just placeholders.
CSS styling is also just temporary placeholders which needs to be replaced when the themes are implemented.
Initially it looks good. Some feedback is to include a screenshot so we can actually see how the navbar looks without having to run the code.
Merge conflict is fine.
What I've reviewed:
The 5 edited files.
Checks performed:
Checked against wiki standards.
Checked if it matches chosen model from #146.
Pulled branch #193 and ran it.
Issues:
The app doesn't compile, app.js pulls from ../components/Navbar/Navbar but the file is located in src/Navbar.
Home() and Settings() should be their own files, I don't think Navbar should define the contents of the page.
Class root2 is not a very descriptive name in app.js.
No comments in JS/CSS file.
Wiki says CSS declarations need to be ordered alphabetically.
CSS also needs comments for each rule set.
Most of the issues has been resolved but comments regarding naming conventions etc will be fixed from the merged branch.
Pull request closed