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.

Sign up for GitHub

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

Jump to bottom

Add dropdown menu to navbar #4188

Merged
merged 3 commits into from Oct 24, 2023
Merged

Add dropdown menu to navbar #4188

merged 3 commits into from Oct 24, 2023

Conversation

falbru
Copy link
Contributor

@falbru falbru commented Oct 13, 2023 •
edited

Description

Adds a simple dropdown menu that appears when you hover over the navbar.

It lacks animations, like in Ivar's original PR, but I was told to be agile so I'm just throwing this PR out there. I tried to add an identical animation, but it was literally mission impossible ;(

Result

Before

Screencast.from.2023-10-13.22-53-19.webm

After

Light mode:

Screencast.from.2023-10-14.11-13-56.webm

Dark mode:

Screencast.from.2023-10-14.11-14-16.webm

Testing

  • I have thoroughly tested my changes.

Everything works... EXCEPT this tiny bug that I encountered. It is not caused by the changes in this PR tho, so it will need to be fixed in a separate PR.

Resolves ABA-143

@linear
Copy link

linear bot commented Oct 13, 2023

ABA-143 New header

@github-actions github-actions bot added the review-needed Pull requests that need review label Oct 13, 2023
ivarnakken
ivarnakken requested changes Oct 13, 2023
View reviewed changes
Copy link
Member

@ivarnakken ivarnakken left a comment

Choose a reason for hiding this comment

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

Thanks for picking this up! It looks nice!

app/components/Header/Navbar/ItemList.tsx Outdated Show resolved Hide resolved
app/components/Header/Navbar/Item.css Outdated Show resolved Hide resolved
app/components/Header/Navbar/Item.css Outdated Show resolved Hide resolved
@ivarnakken ivarnakken added enhancement Pull requests that make enhancements, instead of just purely new features changes-requested Pull requests with requested changes labels Oct 14, 2023
@falbru
Copy link
Contributor Author

falbru commented Oct 14, 2023 •
edited

NEVERMIND. I can just use the additive-color like Ivar suggested.

I think the back ground color of a highlighted item looks a bit weird on darkmode, t looks better when I change the background color from color-gray-1 to color-gray-2 in my opinion However, that color looks horrible on light mode. Do you have any ideas on what to do? Could I make my own color, or should we just leave it be?

itsisak
itsisak approved these changes Oct 18, 2023
View reviewed changes
Copy link
Contributor

@itsisak itsisak left a comment

Choose a reason for hiding this comment

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

Looks great! Well done💯

ivarnakken
ivarnakken approved these changes Oct 24, 2023
View reviewed changes
Copy link
Member

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

app/components/Header/Navbar/EventsDropwdown.css
Comment on lines +1 to +3
.items {
display: flex;
flex-direction: column;
Copy link
Member

Choose a reason for hiding this comment

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

This seems to be unused

app/components/Header/Navbar/ItemList.tsx

const ItemList = ({ items }: Props) => {
return (
<Flex column={true}>
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
<Flex column={true}>
<Flex column>

Same thing

@ivarnakken ivarnakken added approved Pull requests that have been approved and removed changes-requested Pull requests with requested changes labels Oct 24, 2023
@falbru falbru merged commit 687d756 into master Oct 24, 2023
4 checks passed
@falbru falbru deleted the improve-navbar branch October 24, 2023 22:47
@norbye
Copy link
Member

norbye commented Oct 25, 2023

Found a smol bug(: on mobile, the navbar is still displayed.. so you might've gotten a mail that it has been deployed, but I'm only deploying until the commit before this one right now

Visual of the bug; (should've only displayed the logo and the icons, not the navbar text)
image

@falbru
Copy link
Contributor Author

falbru commented Oct 25, 2023

@norbye Good catch! Seems like I forgot to import variables.css in the css file 🙃. I'll fix this right away

@falbru falbru mentioned this pull request Oct 25, 2023
Merged
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@itsisak itsisak itsisak approved these changes

@ivarnakken ivarnakken ivarnakken approved these changes

Assignees
No one assigned
Labels
approved Pull requests that have been approved enhancement Pull requests that make enhancements, instead of just purely new features review-needed Pull requests that need review
Projects
None yet
Milestone
No milestone
4 participants
@falbru @norbye @itsisak @ivarnakken