-
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
Improve mobile inserter and post settings #1089
Conversation
This fixes scroll bleed when post settings is open on mobile. It also fixes the vertical position of it. The downside here is that you lose your place in the editor when you open modal. This is similar behavior to WordPress.com, and it's an improvement on the old editor where you'd have to scroll to the metaboxes below the editor regardless. But it might be worth looking into fixing this with JS down the road.
editor/sidebar/style.scss
Outdated
@@ -27,7 +32,27 @@ | |||
|
|||
/* Visual and Text editor both */ | |||
.editor-layout.is-sidebar-opened .editor-layout__content { | |||
margin-right: $sidebar-width; | |||
margin-left: -200%; | |||
float: left; |
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.
Mixed spaces/tabs indentation :)
The gap at the end is due to some sticky positioning combined with I think this is best addressed separately, as part of #656. |
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 👍
Thank you! |
This PR does a number of things:
The only downside to this PR is that if you are on mobile, and are editing a long document, then open post settings, you lose your place (i.e. you are scrolled to the top). This is actually the same behavior that's on WordPress.com, and it's not really a regression from the current editor where you'd have to scroll to the bottom where the metaboxes live regardless.
But depending on how we deal with the inspector on mobile, we might want to separately address this positioning issue using JS. Potentially we could do that as part of fixing #656.