-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Restyle backend UI flash message to not overlap buttons #5540
Conversation
the common form pattern in the solidus admin is to have the buttons be centered at the bottom. when the flash message would display, it would cover up those buttons before it fades, leading to a bit of a slow admin process waiting for the flash to go away due to it covering key buttons this revises the styles so the flash message shows in the lower right like a little toaster, out of the way of most buttons on a desktop monitor this change comes by way of using the solidus admin daily and wishing the flash didn't overlap the buttons this only applies to the legacy UI, as the new UI no longer has this problem
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.
Hey @brettchalupa, thanks for the contribution.
This is an improvement and I'm also ok with this as is. Was just wondering if might make sense to take advantage of the change to make the flash style as close as possible to the new admin style of the same element.
This might help making the transition between pages in the admin smoother. If you have any idea about how to do it that would be great otherwise, as I said, I'm also ok with this.
Thank again!
@kennyadsl thanks for your feedback! I've thought about it a bit, and I think since this is really just a temporary improvement for a UI that's going away and being replaced, I'd prefer to just do what's quickest and simplest with what's in this PR. What do you think? |
The legacy admin will continue to be used by a lot of stores for a very long time. Many stores have a heavily customized admin to fit their needs or even build their own internal tools with the Solidus admin. Also they might use extensions that have not yet been updated (and maybe never will). The new admin is a complete different architecture and updating heavily customized admins will take a lot of effort. So, I don't see the legacy admin going away soon (if at all). That said: Yes, it is worth your effort :) PS: Though, I do not think we should adopt the centered placement of the new admin tooltip for the legacy admin, since it still will sit on top of the buttons. Maybe just adjust the color to the new admin theme? |
@tvdeyen thanks for all of that context, great to know! I'll work on revising the styles of the flash message then. 👍 |
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.
@brettchalupa I'm merging this since we can release, thanks for fixing it 🙌
Can you handle any further style change in a different PR?
Summary
the common form pattern in the solidus admin is to have the buttons be centered at the bottom. when the flash message would display, it would cover up those buttons before it fades, leading to a bit of a slow admin process waiting for the flash to go away due to it covering key buttons
this revises the styles so the flash message shows in the lower right like a little toaster, out of the way of most buttons on a desktop monitor
this change comes by way of using the solidus admin daily and wishing the flash didn't overlap the buttons
this only applies to the legacy UI, as the new UI no longer has this problem from my testing locally. absolutely no worries if this isn't worth merging since it is only for the legacy UI!
also, this is just a proposal! no worries if the style isn't quite right
screenies
here's what it looks like:
close up
before
here's what it used to be like before these changes, where it's covering some key form buttons
Checklist
Check out our PR guidelines for more details.
The following are mandatory for all PRs:
The following are not always needed: