-
Notifications
You must be signed in to change notification settings - Fork 62
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.
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
feat(ControlStrip): Moved menu button to ControlStrip #605
Conversation
✅ Deploy Preview for volview-dev ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify site configuration. |
@PaulHax what do you think of the placement? The alternative would be to remove the |
Left of logo would look good to. Strangely I don't have a strong opinion =) Just a slight unease about the "connection" between the button and the drawer, but no obvious solution. Overall I think this is a nice upgrade. |
I decided to put it there, because initially it was left of the logo, but I didn't see it immediately, and I thought it would open another menu. I thought putting on the menu, so you can't miss it. |
Try reverting your changes and drop the |
I could put it next to the logo if you think it's better. However, like I said, it was initially there, but it wasn't directly clear to me what was this button for, maybe because of the logo right next to it, I'm not sure. I felt like it was gonna open another dropdown menu or something like that. |
I think this is consistent with how some existing designs operate. The material design website and vuetify's website both use the triple bar icon around the logo to toggle the sidebar visibility on smaller screens. In this case, we are just enabling that functionality regardless of screen size. |
Related to #604