Skip to content
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

Editor Layout: open sidebar menu over editor on small screens #29955

Merged
merged 1 commit into from
Mar 22, 2021
Merged

Editor Layout: open sidebar menu over editor on small screens #29955

merged 1 commit into from
Mar 22, 2021

Conversation

creativecoder
Copy link
Contributor

Description

When a sidebar menu is opened on small screens, open the menu on top of the editor content, rather than squishing the content. This mainly applies to the widgets editor.

Before After
Screen Shot 2021-03-17 at 11 04 18 Screen Shot 2021-03-17 at 11 01 59

Additionally, remove the negative margin that causes the content to jump when the menu is opened in the page/post/site editors.

Before After
open-mobile-menu-before open-mobile-menu-after

How has this been tested?

On small screens (that show the mobile version of wpadminbar)

Widget editor

  1. Activate a non-block theme
  2. Navigate to Appearance > Widgets
  3. Test opening and closing the wp-admin navigation sidebar menu

Post/page/site editor

  1. Navigate to the editor
  2. Test opening and closing the wp-admin navigation sidebar menu

Types of changes

Bug fix, more or less

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • I've tested my changes with keyboard and screen readers.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
  • I've updated all React Native files affected by any refactorings/renamings in this PR. (n/a)

@creativecoder creativecoder self-assigned this Mar 18, 2021
@creativecoder creativecoder added the [Feature] Widgets Screen The block-based screen that replaced widgets.php. label Mar 18, 2021
@jasmussen
Copy link
Contributor

Thanks for the PR! This seems to work well for me and fix the issue. Though looking at when/why those mixins were introduced in the first place, there seemed to be a good reason. See #21661 and #19011. I could'nt reproduce the original issue so it may be moot, but be sure to test with and without fullscreen mode.

@creativecoder
Copy link
Contributor Author

Though looking at when/why those mixins were introduced in the first place, there seemed to be a good reason. See #21661 and #19011. I could'nt reproduce the original issue so it may be moot, but be sure to test with and without fullscreen mode.

Thanks for that context @jasmussen ! I double checked small and medium screen sizes and toggling full screen mode when available: I also was not able to reproduce the original issue in #19011.

Copy link
Contributor

@jasmussen jasmussen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's try it.

Copy link
Member

@david-szabo97 david-szabo97 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 🚢 ✅

Copy link
Member

@vindl vindl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works as expected! :shipit:

I noticed there is a similar issue with Navigation screen (needs to be enabled in Gutenberg experiments) that would be a nice follow-up item.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Widgets Screen The block-based screen that replaced widgets.php.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants