Redesign header and navigation

[owi92]

This applies the suggested redesign of the header and navigation components.

Notes:

• There is still some stuff missing, which I just noticed 🤦, and also some aspects that can be improved upon, so expect some more commits/amends in the following days.
• Also there are some design related things I had to decide myself, because they were missing in the suggestions. I'd be happy to change them, and anything else, based on further suggestions.
• I tried keeping the color scheme neutral, as this is yet to be overhauled and should also be configurable, but I'm not necessariliy convinced that my choices fit in that well within the default scheme.
• Last but not least I'm sure there are also some things code-wise that can be improved or optimized that I either missed or don't know enough about yet.

[github-actions]

This pull request has conflicts ☹
Please resolve those so we can review the pull request.
Thanks.

[owi92]

Well... somehow I failed again to do a proper rebase to make github's compare button useful. Also I did't manage to merge both our changes in UserBox.tsx without breaking everything while doing the rebase , so I went with my file for now. I will try to incorporate the FloatingMenu in the future.

Edit: Actually, I am not sure if we even want to use the FloatingMenu for the user box. It really depends on how close we want to match the design files.
But I think at least the language menu on desktops could profit from the little arrow tip, so I guess I will try to add it there first.

[owi92]

I believe this is ready to be reviewed now. There are only two things still bugging me (apart from possible code style things and all css being spread over so many files): The menus (language, user, burger) do not trap focus. And they still dont use the FloatingMenu that was introduced recently. But since I already spend so much time on this, I think I should be moving on to the remaining design issues first.

[LukasKalbertodt]

Thanks for the changes! It already looks good and I like a lot about it. Especially the nav menu looks and feels good (The hover effect with out but without in-transition is great). I also like the indentation of the nav items in the burger menu.

I have not yet looked at the code changes! I will do that later. But I have comments about the design. Despite how they are formulated, this is all just opinion and stuff we can talk about. Oftentimes, I'm not sure myself. And lots are about Lisa's design, not your implementation of it.

General

• I think I would chose a less strong hover style for the search input and all buttons (including login). It's fine for :focus, but I would make it more subtle for hover. For the login button, the background color change on hover is sufficient I'd think.
• I would not use any blur on the box shadow on hover.

Logo

• The logo could use a "To homepage" (or something similar) tooltip when hovering or focusing.
• The logo is quite small now. But that's something that only has to do with our specific header height and logo margin. The latter of which we should just reduce (I can do that if you'd rather not touch the SVG; its important to keep the margins for both logo versions exactly the same).
• Check logo margin/position calculations

Search

• Keep the old search icon
• Below 880px when the search input is hidden and the search button is shown, I would always put the search button leftmost so that it's closest to where the search bar would have been. I realize that Lisa suggested it differently but yeah, my intuition would say leftmost.
• Why margin-right: 15 on the search input? I don't see the need for it and it causes the hover effect even when the mouse cursor is right of the visible input.

Login

• Login button seems very big. Is that just me? I guess it's according to the design? Mhh. Or maybe we want to not make it not have this strong background color? Mhhh.
• The small version of the login button does nothing when clicked.
• The big login button has a transition for the background color, but not the box shadow. If the box shadow is not used on hover anymore, its less of a problem. I would still probably remove the transition-in, but have a out-transition (i.e. when starting to hover, change bg color immediately, when stopping to hover, have transition).

Language

• The language menu could indeed use the triangle-pointy-thingy from our normal popovers.
• The language icon could be a touch smaller? Mhh. Well, when the other icons are shown, it looks fine. But on Desktop view with only the login button beside it, it looks a bit big? Dunno.
• Maybe close the language menu when having chosen a new language?

Navigation/Burger menu

• The navigation burger menu should never be full width, but always show a bit of the page peaking through, as it is right now in the old design.
• The border around the burger menu icon should be thicker I think, according to Lisas design
• Maybe the arrow to get to the parent page in the burger menu should rather be a 90° curved arrow (corner-left-up in feather icons). With the current arrow one might have the intuition that clicking it closes the burger menu.

User menu

• The user popover doesnt quite align with the user box. I know it's difficult because the username varies in length. But it should either be aligned on the right or centered. Oh, I just saw that it's like that in Lisa's design. I don't know why and I still think it should be changed :D
• The pointy thingy inside the user box could be a bit more to the right if you ask me. Oh, Lisa said so. This is becoming a recurring pattern eh? :D Well, I can state my preference

[owi92]

Ok so I have updated the things you commented on excluding the logo.
I couln't make it work with the FloatingTooltip thing and also had some problems understanding how the margin and positioning works, especially considering that there will be other logos depending on the production build. Maybe we can take a look at that together at some point.
I also tried refactoring some stuff, mainly css, but I think there are still some funky/hacky expressions floating around in there.

Some more notes:

• I really wasn't sure about your comments regarding transitions. If I remember correctly, I didn't even add any. So they might be remnants of some of your code? IDK :D

• The pointy thingy inside the user box could be a bit more to the right if you ask me.

• Are you sure about that? From my point of view the gap between username and chevron already seems pretty large. Of course I'm open to increasing it even further if you insist. Or did I misunderstand your comment somehow?

[LukasKalbertodt]

Ok I did another round of testing the test deployment (not looking at code). All previous points (except the one you mentioned) are indeed solved. I found a few new ones, but it already looks much better!

Language menu

• Cursor on active language should be the default, not suggesting you can click it.
• Language menu design doesn't really convince me (active background too dark?). Why not simply use same design as the user menu does?

User menu/button

• In user menu: hover background is same grey as shadow -> looks weird. Make shadow darker? Or hover background lighter? Or both?
• Display name can wrap into two lines when there is not a lot of space, so I'd add white-space: nowrap.
• Header doesn't work so well for long user display names. User buttons should have a max width (e.g. 230?) and text-overflow: ellipsis. Text overflow on large screens can be larger. Not sure how to best do it.
• Logging out makes my Bitwarden ask whether it should remember the password?! Can you reproduce it? If not, I can debug that.

Probably just Safari weirdness

• On Safari mobile the search, language and burgermenu icons are... blue? Not the login button tho
• Border radius of burger menu button does not work on Safari mobile?!
• Safari mobile: border radius on search field selection doesn't work

Other

• Space between menu and content could be smaller. Reducing margin-bottom to 24px is totally fine IMO. Especially on mobile, but maybe on desktop too, reducing that space further seems like a good idea. (E.g. 16px).
• Is max width wider? Whole page seems wider.
• "User features" is not translated into German. Also not sure if it's the best description. Couldn't we leave that space empty instead? Or "user menu"? Or just "user"?

---

excluding the logo.
I couln't make it work with the FloatingTooltip thing and also had some problems understanding how the margin and positioning works, especially considering that there will be other logos depending on the production build. Maybe we can take a look at that together at some point.

Certainly. We can do that when most other stuff is done.

The pointy thingy inside the user box could be a bit more to the right if you ask me.

Are you sure about that? From my point of view the gap between username and chevron already seems pretty large. Of course I'm open to increasing it even further if you insist. Or did I misunderstand your comment somehow?

What I meant: the pointy thing almost touches the blue outline of the user button. There is maybe 1 or 2 px between that. I would slightly increase that so that it doesn't look as if it is touching the outline (i.e. move the floating down). OR we can also move it closer (i.e. move the floating up) so that the pointy thingy overlaps the user button slightly. I think both is fine, only "barely touching" looks weird.
But I also noticed that the shadow is partly to blame here as it gets really weak towards the top. Maybe making the shadow slightly darker as suggested above already helps, but maybe I have to check adjust the floating shadow code at some point.

[LukasKalbertodt]

Okay, finally managed to look at the code. I reviewed everything except navigation related changes. But since it's late already, I just wanted to push this review out before my weekend.

[owi92]

Updated things based on your comments and did some smaller additional changes, mostly moving stuff around and cleaning things up a bit.
I also tried to make the css of the navigation/burger menus a little more sensical by replacing the css classes I used there before with regular selectors. This could have been solved with media queries, but with those there would be a sudden change of the burger menu's style when/if users resize their screens with the burger menu open.

Regarding your top level comment:

Language menu

• I changed the language menu to feature the same styling as the user menu. This is also applied to the larger mobile/tablet menu which means it doesn't comform with the redesign anymore.
  The design only provides a styling for the user menu on desktop and only one for the language menu on mobile, so I debated whether those stylings should be applied to the other menu respectively. But I don't think the language menu style would work that well for the user menu, so I decided to stick with one style for both menus and all sizes.

User menu/button

• That bitwarden thing you noted sounds weird, but I can't imaginge or at least not see how this would be caused by my changes, as I didn't change the way the logout button works. Does this only happen on this PR? Also could not reproduce it, but I don't use bitwarden, so there's that...

Other

• I kept the wider layout for now, as this was suggested by the design. But I don't mind the previous more narrow version. I'd like to leave this decision to you.
• Instead of user I decided to show the actual username as menu description, to somewhat mirror the way the menu is presented on desktop (with the button showing the username). But of course this could also say "user", if you prefer.
• FloatingMenu distance/shadow/hover background: I increased the distance a little and made the hover background lighter (now uses grey97 like in the old design). I did however not adjust the shadow, as a darker shadow does look kinda weird, but maybe that's just me. Well I guess it could be a little darker, I don't really have a hard opinion on that.

finally, Safari

• Safari is giving me a headache.
• Everything is weird on Safari. At least the thing that some components can't be focused by tab navigation. I think this might be caused by using the :focus-visible pseudo class, which does not appear to be supported by safari. We might have to change this to :focus. I'll try tomorrow.
• Border radius + outline not working seems to be a known issue, there is a workaround but I haven't had the time/patience to try it yet. Could also potentially use some other means of doing the outline (i.e. with border or box-shadow). This is an issue on multiple elements.
• I can't reproduce the blue thing, but I might need to update. But login icon-button behaving differently might be because it isn't using the ActionIcon component like the other icon-buttons. Might have to change this.
• I can't even login to tobira on localhost (no matter which branch I'm on). Maybe some setting on my end? Idk.

Sorry for the long comment and many edits on the shorter ones! I guess I kinda went down the rabbit hole here.
One last note: Before this eventually gets merged, I think I should squash some commits, as many are outdated and some now have inaccurate commit messages (and there are too many commits overall in this).

[LukasKalbertodt]

Getting close!!

• I would change the blur radius of the floating thingies to 8px. I think that separates the floating thing better from the background.
• We can ignore Safari for now, but should maybe create a general "check & fix safari" issue

[LukasKalbertodt]

By the way, what do you think about removing the "current site" from the nav on desktop? Something like this?

We already noticed a while ago that the current page title is shown in many different places. Is it really that important in the desktop navigation I wonder? 🤔

[LukasKalbertodt]

🎉 Took a while but the better design was worth it!

Just for reference, these things still need to be done after merging this:

• Work on logo margin/positioning and add a tooltip saying "to homepage"
• Refactor navigation to be defined in a data structure that is passed to the burger menu and side box.
• Find and fix problems in Safari
• Fix the lackluster shadow around the floating arrow tip
• Take a look at width of the three main header elements (flex grow, basis, ...). Search box should be able to shrink somewhat and stuff.
• Again ask around on whether closing menus on blur is a good idea for accessibility.
• Collect feedback on whether the current page name should be hidden in the desktop navigation.

For ease of planning: I will Initiative all those things. So no need for you to do anything until you hear from me.

Thanks for sticking with this long review process for so long!