-
Notifications
You must be signed in to change notification settings - Fork 4.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
Fix/block toolbars with preview #20351
Conversation
Thanks for your help on this @tellthemachines we're almost there Something is off in the style of the top toolbr for mobile/table previews I believe that's most likely because some styling that are meant for contextual toolbars are bleeding to the top toolbar in these previews. The mix might be just moving some styles around. |
Should be fixed now! |
Thanks, it's better I still see the black borders though, Maybe some wrong CSS. selector not targetting the contextual toolbar. specifically? cc @jasmussen |
Interesting, the off-borders only happen when the "Top Toolbar" option is not toggled. When it is, they look correct: In other words, the difference is a classname: So, to the div that has the following classnames:
... if you add this, it works:
|
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.
@tellthemachines While checking this PR, I was wondering something, why the "block-editor" package has to know about the "previewDeviceType" at all? Why it's not just an "edit-post" or "editor" thing?
(That's not something for this PR obviously)
Edit: follow-up here #20409
@jasmussen while this class would have fixed it, I don't think it was the right fix. Instead, I updated the block-toolbar styles so that the "contextual" style override the default lighter borders instead of the opposite to avoid using something from |
There used to be a separator: Compare with this screenshot, there was a separator before the mover control. And there's an issue when you don't have the top toolbar option enabled, where the smart mover thing does its autohide stuff. |
Good catch, these should be fixed now. |
💪 |
Description
Fix rendering of block toolbars with responsive preview by always fixing them to top on Tablet and Mobile previews.
How has this been tested?
Browser tested only.
Screenshots
Types of changes
Checklist: