Andras/BoundlessFlowCampus2K

Fork 0

#193 Navigation Bar #212

Closed

b24jonel wants to merge 0 commits from #193-Navigation-Bar into team_1_week_1

b24jonel commented

2026-04-09 13:47:44 +00:00

Collaborator

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.

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.

b24jonel added 1 commit

2026-04-09 13:47:44 +00:00

#193 Navigation Bar ec8366badc

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.

b24jonel was assigned by b24johka

2026-04-09 13:50:36 +00:00

b24johka commented

2026-04-09 13:53:36 +00:00

Collaborator

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.

b24johka self-assigned this

2026-04-09 13:53:55 +00:00

b24jonel commented

2026-04-09 13:54:07 +00:00

Author

Collaborator

No description provided.

image.png

27 KiB

c24elipe added the

needs review

label

2026-04-10 06:21:36 +00:00

c24carso was assigned by c24elipe

2026-04-10 07:49:22 +00:00

b24johka commented

2026-04-10 08:43:53 +00:00

Collaborator

Merge conflict is fine.

c24carso commented

2026-04-10 10:17:30 +00:00

Collaborator

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.

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.

b24johka commented

2026-04-10 10:33:03 +00:00

Collaborator

Most of the issues has been resolved but comments regarding naming conventions etc will be fixed from the merged branch.

b24johka closed this pull request

2026-04-10 12:49:30 +00:00