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

Remove sticky-menu class preventing full admin page scroll #11308

Merged
merged 3 commits into from Nov 15, 2018

Conversation

Projects
None yet
4 participants
@johngodley
Contributor

johngodley commented Oct 31, 2018

If you have a long admin menu it's not possible to scroll down when Gutenberg first loads.

edit_post_ wordpress_latest wordpress_and_gutenberg fix_fullscreen-sticky-menu__8244_commits__and_gutenberg_index_js _gutenberg

This is caused because of #9567 adding is-fullscreen-mode to the page body class. This causes core WordPress to add sticky-menu to the page, which sets the admin menu to position: fixed.

Resizing the window causes Gutenberg to re-apply body class values, and sticky-menu is then lost.

More information can be found in this comment.

This PR modifies the FullscreenMode component to remove sticky-menu when it first starts to counteract adding is-fullscreen-mode in PHP.

Fixes #9996

How has this been tested?

  1. Create plugin described here to extend your admin menu
  2. Verify that on page load it's possible to scroll to the bottom of the admin menu

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.
@johngodley

This comment has been minimized.

Contributor

johngodley commented Oct 31, 2018

Noting that not adding is-fullscreen-mode in PHP also works, but then there is a noticeable UI jump when a page is first loaded.

I'm also not exactly sure if this is the best place and I appreciate the component may not be specific to WordPress.

Remove sticky-menu class preventing full admin page scroll
Gutenberg adds ‘is-fullscreen-mode’ to the page body class, causing WordPress to add ‘sticky-menu’. This prevents the page being vertically scrolled, cutting off long admin menus.

Remove ‘sticky-menu’ as part of the FullscreenMode setup

@johngodley johngodley force-pushed the fix/fullscreen-sticky-menu branch from 1e4a309 to 4baf632 Nov 1, 2018

@johngodley johngodley requested a review from WordPress/gutenberg-core Nov 6, 2018

@tofumatt

I think this needs to add the class back when fullscreen mode is left, and this probably warrants a test too.

@noisysocks

This comment has been minimized.

Member

noisysocks commented Nov 14, 2018

Hi @johnbillion! What's the status of this PR?

@tofumatt

This comment has been minimized.

Member

tofumatt commented Nov 14, 2018

I think you meant @johngodley 😉

@noisysocks

This comment has been minimized.

Member

noisysocks commented Nov 14, 2018

🤦‍♂️

@johngodley

This comment has been minimized.

Contributor

johngodley commented Nov 14, 2018

What's the status of this PR?

I need to address the point raised above - will circle back.

johngodley added some commits Nov 14, 2018

Restore sticky-menu class if it previously existed
In case it was added for a legitimate reason originally.
@youknowriad

LGTM 👍 And thanks for the tests.

@johngodley

This comment has been minimized.

Contributor

johngodley commented Nov 15, 2018

Do the changes look ok to you @tofumatt ?

@tofumatt

Works for me!

@youknowriad youknowriad added this to the 4.4 milestone Nov 15, 2018

@youknowriad youknowriad merged commit a356318 into master Nov 15, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@youknowriad youknowriad deleted the fix/fullscreen-sticky-menu branch Nov 15, 2018

grey-rsi pushed a commit to OnTheGoSystems/gutenberg that referenced this pull request Nov 22, 2018

Remove sticky-menu class preventing full admin page scroll (WordPress…
…#11308)

* Remove sticky-menu class preventing full admin page scroll

Gutenberg adds ‘is-fullscreen-mode’ to the page body class, causing WordPress to add ‘sticky-menu’. This prevents the page being vertically scrolled, cutting off long admin menus.

Remove ‘sticky-menu’ as part of the FullscreenMode setup

* Restore sticky-menu class if it previously existed

In case it was added for a legitimate reason originally.

* Add unit test for fullscreenmode component
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment