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

WP admin doesn't scroll when on gutenberg screen #4445

Closed
m-e-h opened this issue Jan 12, 2018 · 21 comments · Fixed by #4629
Closed

WP admin doesn't scroll when on gutenberg screen #4445

m-e-h opened this issue Jan 12, 2018 · 21 comments · Fixed by #4629

Comments

@m-e-h
Copy link
Member

m-e-h commented Jan 12, 2018

Issue Overview

I'm unable to scroll the wp-admin sidebar up or down. So I can't navigate to or view all of my post-types and menu-items.

Steps to Reproduce (for bugs)

  1. Have taller sidebar menu than the view-port.

Chrome 65 and Firefox 57 on Ubuntu 16.04

Expected Behavior

Scroll the admin sidebar on the y-axis.

Current Behavior

Doesn't scroll.

Possible Solution

Rather than changing the overflow behavior on the entire wp-admin by declaring on the document body like this https://github.com/WordPress/gutenberg/blob/master/editor/assets/stylesheets/main.scss#L1-L8
Declare it on just the editor.
I'm not clear on what's being accomplished with this CSS but maybe add it somewhere like .gutenberg or #wpbody?

Screenshots / Video

gut-bodyscroll

@m-e-h
Copy link
Member Author

m-e-h commented Jan 12, 2018

Here are some better examples of the issue I'm seeing. (I have a ton of post-types)
These gifs include exactly what I'm seeing on my monitor. Nothing is cut off on the bottom.

Current CSS with overflow: hidden on the body:
scroll
I'm unable to scroll the rest of my left sidebar into view. And notice that the editor's scrollbar actually goes out of view, leaving it unknown where you are in the content.

Here is the overflow: hidden removed:
scroll-jank
I'm able to scroll all of the sidebar into view, however, the scrollbar still disappears. I also notice the jank that was mentioned in this pr 353ee9c

Here, overflow: hidden is removed and I've added position: relative to .gutenberg since it wraps .gutenberg__editor which is position: absolute:
scroll-full
I'm able to scroll and the editor scrollbar stays in view.
The editor window does get a little out of sync since the document body continues to scroll the height of the sidebar once the editor has scrolled to it's end.
There's got to be a way to stick that editor to the bottom of the viewport. I'll have to play with it some more over the weekend.

@bamadesigner
Copy link

I am having the same issue. If my viewport is shorter than my admin content, I can't scroll outside of the Gutenberg edit area and therefore can't access my full admin menu.

@Micemade
Copy link

Same / similar issue - referenced above

@m-e-h m-e-h mentioned this issue Jan 18, 2018
3 tasks
@m-e-h
Copy link
Member Author

m-e-h commented Jan 19, 2018

I added a fix for this in pr #4587

Adding the scrollbar to the admin-menu seemed like the cleanest fix and won't even be noticeable to anyone who isn't currently having this problem of not being able to scroll.. It's only applied when the body gets the overflow: hidden (> 600px), so it shouldn't mess with mobile.

I tried to tie the menu in with the body scroll, but just couldn't make it smooth. Also didn't know if we were purposely keeping the scroll off of the body?

I'm just now diving into Gutenberg so I may have missed previous discussions on the layout behavior.
@jasmussen looks like you've spent some time with the layout. Is there a reason we wouldn't want the menu to scroll independently? Is there a better way?

@jasmussen
Copy link
Contributor

Thanks so much for working on this. There are two key issues that we want to make sure doesn't regress:

  • Scroll-bleed from the inserter. Try opening the inserter on a very long document, then click the "Blocks" tab and mouse-wheel to the end of the blocks tab. Verify that once you've scrolled to the end of that you don't start scrolling the document below.
  • Mobile breakpoints. Verify that this doesn't regress the mobile scrolling behavior compared to master.

From a quick glance, it looks like #4587 seems to both work well, and not introduce any issues with the above two, as far as I can tell.

I'm going to take a look again next week and see if there is a more elegant way to do it, but my gut tells me there isn't, without dropping the individually scrolling main content area (which would re-introduce the scroll bleed). But I'll get back to you, and thank you again!

@m-e-h
Copy link
Member Author

m-e-h commented Jan 20, 2018

Ahh OK, that makes sense.
Yeah, I'm not seeing #4587 have any affect on scroll bleed or mobile or anything else really except for the admin-menu. Which I'm actually kind of liking with a scroll.

One thing I have noticed with #4587 is that the scrollbar cuts into the width that the menu-items have to work with.
So if a menu-item title is maxing out its 160px , it'll get a letter or two cut off. That's with the overflow-x: hidden I put on the menu-wrap. Otherwise a long title will add a horizontal scroll. Both suck as far as backward compatibility.
I haven't tried to fix that part yet. Maybe something can be done with box-sizing or maybe add a couple extra pixels if we have to.

@jasmussen
Copy link
Contributor

Took another look at the PR, and realized that the reason why there's a horizontal scrollbar, is that the flyout menu has to have space to fit there. Incidentally, our tweaking of overflow breaks that.

Before:

before

After:

after

This is a bummer, and means we'll have to take a different approach to addressing this. Thanks again for working on it, though.

jasmussen pushed a commit that referenced this issue Jan 22, 2018
This fixes #4445. Props @m-e-h to starting the work to address this.

This removes `overflow: hidden;` from the `body` element, which was previously applied at the desktop breakpoint. That means if you have a very tall navigation menu, or your viewport isn't very tall, then you can at least scroll the main container to reveal all menu items.

This is not an ideal fix, as it adds another scrollbar in this edgecase. But given it's an edgecase, and given the benefits of preventing scroll bleed the individually scrolling main content area has, perhaps it's worth it?

We need to test this PR to see that there aren't other side effects.

We also tried letting the nav container itself scroll, but that crops the flyout menu.
@marcusig
Copy link

@jasmussen

But given it's an edgecase

I'm not sure it's an edge case. On my laptop with a few plugins installed which add:

  • a few menu items
  • metaboxes (Woocommerce)

Both the menu bar, the editor's content and the metaboxes get cropped if body { overflow : hidden; }.

I've experimented a bit and found a couple of ways to stop the scrollbleedin' thing, but not sure what's best...

@jasmussen
Copy link
Contributor

Note to self: never use the term edge case 😉 it clearly is an issue that needs fixing, whether edge or not.

Incidentally I submitted a PR earlier today that seems to address it: #4629

@marcusig
Copy link

haha :)
Your PR removes the overflow: hidden isn't it?
How about this issue you mentioned?

Scroll-bleed from the inserter. Try opening the inserter on a very long document, then click the "Blocks" tab and mouse-wheel to the end of the blocks tab. Verify that once you've scrolled to the end of that you don't start scrolling the document below.

@jasmussen
Copy link
Contributor

Yep, that's the thing, when the menu needs to scroll, there is a little scroll bleed. When it doesn't, there's no scroll bleed. It's not an ideal fix, and we'll probably want to test thoroughly on metabox heavy configs, but I can't think of another fix that doesn't involve killing the flyout menu.

@marcusig
Copy link

I fiddled a bit in the inspector and found a way to stop the scrollbleed with css, and looked at a JS solution as well.
The JS idea: https://jsfiddle.net/0o2y3uw9/
With CSS, if you leave the overflow: hidden on the body and add
#wpwrap { overflow: auto; height: 100%; }
It sort of moves a few things around which would have to be fixed as well.
One of the cons is that the scrollbar for #wpwrap is hidden (the sidebar sits on top), which can be an accessibility issue (yes, some people actually click and drag the scrollbar).

@m-e-h
Copy link
Member Author

m-e-h commented Jan 22, 2018

Another JS approach would be to toggle a body class when the inserter is open and hide the overflow only while open.

body.inserter--is-visible {
    overflow: hidden;
}

@marcusig
Copy link

Could work, but there might be some movement when changing the overflow (showing and hiding the body scrollbar)

@jasmussen
Copy link
Contributor

I fiddled a bit in the inspector and found a way to stop the scrollbleed with css, and looked at a JS solution as well.

Appreciate your help. I've fiddled myself many times:

But each of those come with their own tradeoffs, and none of them fit the wp-admin and its legacy code very well. The same with hiding the scrollbar, it causes that page-jog.

As such, I honestly feel like what we have now, with some scroll bleed in the case of long admin menus, is the most acceptable of the various tradeoffs.

I also don't see any issues with the metabox implementation. This feels issue, to me, appears isolated to the left hand navigation pane. Here's a GIF of the overflow-removing PR:

scrollbleed

I acknowledge it's not ideal to see the two scrollbars on the right, on short viewports, but it feels like it is the least disruptive patch, and works reasonably well. Also remember that on mobile we're addressing this in an entirely different way.

@marcusig
Copy link

Fair enough. It's also probably the most reasonable and clean code.
Off topic, how do you make those GIF screenshots?

@jasmussen
Copy link
Contributor

jasmussen commented Jan 23, 2018

Off topic, how do you make those GIF screenshots?

For when I need to make long GIFs that aren't insanely huge in filesize (still big, but not crazy), I use Licecap. It uses high compression and low framerates to keep the filesizes down, at the expense of a little choppiness, and light grays disappearing.

When I need to do more high quality GIFs, I used to use Giphy (Mac app store). But now I just screen-record using Quicktime, then open the .mov file in GIF Brewery to convert it to GIF. Finally I run the GIF through ImageOptim to slim down the filesize as much as possible.

@marcusig
Copy link

Thanks. It's quite practical to illustrate all this trials and tests.

@jasmussen
Copy link
Contributor

It's very practical. It is a little frustrating that GIF is still the best format for this, since it's so old and impractical. But it's just works everywhere, so I find it still the best way to convey little interactions.

@mikestopcontinues
Copy link

What about padding the bottom of the gutenberg editor equal to the admin-menu overflow? Then we'd at least see the bottom of the doc.

@mikestopcontinues
Copy link

Actually, is there anything wrong with using fixed positioning for .edit-post-layout__content? Basically, treating it the same as .edit-post-sidebar.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants