Closed fhenrich33 closed 5 days ago
All modified and coverable lines are covered by tests :white_check_mark:
Project coverage is 84.49%. Comparing base (
168e5d5
) to head (5b541aa
). Report is 146 commits behind head on master.
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
@aerinsol could you check out the screenshots from Felipe if it is the way you like it.
FYI: I do not really get why we switch to a top menu for large desktops. There we would have the most real estate to show the drawer menu.
Also is the drawer menu on small desktop/tablet collapsable, meant to be hidden?
FYI: I do not really get why we switch to a top menu for large desktops. There we would have the most real estate to show the drawer menu.
Two reasons. 1. the last time we put this design forward, it was said the drawer menu was too difficult to implement. 2. Sometimes there isn't enough vertical real estate to show things. We see this on small screens for Dropapp.
@aerinsol could you tell me what you think of my suggestions on the design mentioned in the review? I marked them with [for @aerinsol to decide]
- the last time we put this design forward, it was said the drawer menu was too difficult to implement.
Depends a bit, it would have been less work than implementing a third kind of menu. @aerinsol
Ok well, @HaGuesto would have been nice to have that kind of feedback earlier since those designs have been around a very long time and were designed for all possibilities, including transitional menus. In Figma it also looked like the horizontal design wouldn't work for smaller screens. Is there a call to action here or...?
@fhenrich33 it looks great.
Ok well, @HaGuesto would have been nice to have that kind of feedback earlier since those designs have been around a very long time and were designed for all possibilities, including transitional menus. In Figma it also looked like the horizontal design wouldn't work for smaller screens. Is there a call to action here or...?
I apologize for not taking a deeper look into the designs before Felipe took up the epic. There is some behavior though that is hard to fletch out in figma and now is just taken over from the standard components of chakra. I'd like to propose the following changes to the design and would like your opinion (a yes or no on the different suggestions) if any of these should be implemented @aerinsol:
align the visual design of main menu buttons in all screen sizes In the drawer menu the chakra accordion component is used instead of the chakra menu component. That means that visual design as well as hovering and clicking is different: I suggest that
align the visual design of sub menu buttons in all screen sizes I suggest that the background of the main menu buttons is white or slightly gray normally, slightly gray or a bit darker gray when hovered over it.
align the drawer menu border in tablet screen size with the standard design of the accordions At the moment the border of the drawer menu is black and fully surrounding the drawer menu. I'd
All of these are smaller changes and might be best fixed to define a template style enriching the standard chakra Menu component here
Hmm. I had a think about this, and I think keeping the drawer and the mobile menu only, and dropping the top menu is the best solution for consistency. The drawer menu is the most understandable for someone not used to tech and has the best information hierarchy. I spent the most time on it next to the mobile menu.
Yes, I would like the drawer menu to be collapsible (close able in Chakra) and also to have a drop shadow over the main screen (@fhenrich33 you can just pick another component with a drop shadow, if you can't find it we can leave it for a follow up).
I'll go through the options now to see how to align those two.
Also @fhenrich33 good job selecting all the correct icons! I know it's a small thing but I spent a lot of time trying to sort through all of them and label them in Figma correctly 😅
- align the visual design of main menu buttons in all screen sizes
Yes, let's match the drawer color scheme for both menus. White bg for main menu buttons, grey bg for child menu buttons, and red when clicked on or expanded. Font is black except when the background is red, then it's white and bolded (@fhenrich33 you are bolding it onClick
in the drawer menu, right? Keep it to whatever you're doing there.)
- there should be no spacing/margin between the main menu buttons (this is currently only the case in the mobile hamburger menu). Instead we could add the slight gray separation lines from the accordion.
So actually, there are supposed to be 3 separate menu groups in the mobile hamburger menu with <MenuDivider />
or
Also,@fhenrich33, Scan QR Label Menu button should be the same size as all the others, and "Beneficiaries" is spelled incorrectly in all menus.
- align the drawer menu border in tablet screen size with the standard design of the accordions At the moment the border of the drawer menu is black and fully surrounding the drawer menu. I'd
- remove the border from the top, left, and bottom so that the border just becomes a separation line between left and right.
- use the color and thickness of the accordion separation lines to make the border less visually outstanding.
This is fine, as I said earlier I think a drop shadow would be helpful but not a deal breaker for production.
As discussed in Slack and other places, the icon and text for Admin will be adjusted to a lock icon and the text "Coordinator Admin"; Logout will also now have a key icon.
Reason for icon adjustment: It is intended to better mimics offline reality for non-tech people. Something with a lock mimics a locked door or cabinet that only certain people have access to; logging out is similar to taking a key to lock up a shop or a warehouse at the end of the day.
@fhenrich33 Thanks for the update! I'm quite happy with it. One feedback: I generally prefer doing rather global definitions. I'd for example would like to define the screensizes at some point globally or the Menu component instead of inside each component.
@aerinsol There are a few pointers for you to judge since it is not 100% clear to me from the figma:
@HaGuesto
- the behavior of the expanding menu on the large screen should be on topo of the content or the content should be squeezed to the right?
It should be squeezed to the right, and the menu should be open by default. We should probably use arrows to show open / closed - I can add that if needed.
- the account and logout button is still on the top right
Yeah, I'll probably change this in the next iteration. I thought the top right might be nicer but now I see it's probably not.
- the is implemented in mobile screen but not on the large screen
?
- should the accordion in the menu stay open or should always only one accordion be open?
Only one accordion should be open on the mobile menu at a given time.
@aerinsol
It should be squeezed to the right, and the menu should be open by default. We should probably use arrows to show open / closed - I can add that if needed.
Chakra doesn't support squeezing out of the box for Drawers, we would need to roll our own implementation.
See https://v2.chakra-ui.com/docs/components/drawer
Only one accordion should be open on the mobile menu at a given time.
Fixed that right after @HaGuesto comment.
It should be squeezed to the right, and the menu should be open by default. We should probably use arrows to show open / closed - I can add that if needed.
Chakra doesn't support squeezing out of the box for Drawers, we would need to roll our own implementation.
It looks like it does, but only if you purchase Chakra Pro.
https://pro.chakra-ui.com/components/application/sidebars
If that's the case, for the sake of time I am happy to go back to the fixed sidebar style you originally put out @fhenrich33
It looks like it does, but only if you purchase Chakra Pro.
Seems to be only for version 3 onwards. Reverted to fixed!
Desktop/Tablet:
Mobile: