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
only show expand toggle, in view mode, on hover #14706
only show expand toggle, in view mode, on hover #14706
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm - very nice. This really cleans up the dashboard look and feel
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! You're on a roll! 🔥
Have you also considered hiding the ContextMenu buttons when in edit mode, and revealing them on hover?
@cjcenizal , hmmm, I have not, until now. Do you think if I did that we'd need a way to visually indicate edit mode? With the new margins introduced, I got rid of the dashed borders. I could bring them back I suppose, but they are distracting IMO. Maybe we don't need an indication of view or edit mode? |
* only show expand toggle, in view mode, on hover (and focus for accessibility) * Fix tests
@stacey-gammon Oh I didn't notice the dashed borders are gone! I just took another look and I definitely think the old borders were ugly, so it's good that they're gone. But I do think that it's less clear when you're editing and when you're not because the dashboard looks almost identical in both modes. We could solve this by tweaking the "edit" mode slightly. Here I've made the border-style "dotted", border-width 1px, and removed the box-shadow for edit mode: Contrast against the view mode style: Thoughts @stacey-gammon and @snide ? In terms of my original suggestion, once the difference between view and edit mode is more clear I think we can make a better decision about whether hiding the context menu makes sense. |
lgtm, if @snide has no objections, I'll go ahead and implement |
Good catch, though I think we'll need some more definition beyond the shadows. If we can wait, I'll spend a couple hours monday cleaning all this stuff up. There's a lot more easy fixes we can do in there. |
Works for me, thanks @snide! |
* only show expand toggle, in view mode, on hover (and focus for accessibility) * Fix tests
* only show expand toggle, in view mode, on hover (and focus for accessibility) * Fix tests
They are distracting in view mode when they show up on every panel.
Before:
After: