-
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: Move the "View Posts..." anchor out of the role=toolbar section in the header #15693
Conversation
@@ -38,7 +39,10 @@ function Header( { | |||
className="edit-post-header" | |||
tabIndex="-1" | |||
> | |||
<HeaderToolbar /> | |||
<div> |
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.
@kjellr or @jasmussen - I don't know if this is the best idea to wrap them with div but I took as little steps as possible to make it look close to what we had before. I would love to hear your recommendations because I also noticed that the border is slightly broken now:
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.
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.
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.
It looks like if this particular div is assigned display: flex;, and .edit-post-fullscreen-mode-close__toolbar is also assigned display: flex; (instead of inline-block as it is now), both issues are fixed:
Yep, that fixes it for me too. 👍
09d4dc9
to
7511aee
Compare
I applied changes as recommended by @jasmussen and @kjellr, many thanks for your help 🙇 It seems like UI wise everything looks exactly the same now: Branch: Master: Well, there might be a slight difference in the left margin of the inserter. It's hard to confirm 😅 |
Nice work. Be sure to test all mobile breakpoints, and Firefox and Safari. Otherwise, 👍👍 |
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.
Tested in Safari, Chrome, + FF, across a wide range of screen widths. Looks great to me!
Description
Partially fixes #15331.
This PR follows the recommendation:
Testing
Enabled Fullscreen Mode through More Menu item. Ensure that the header looks like before and
View Posts
anchor is outside of the toolbar.Screenshots
Checklist: