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

Clean up toolbar position for wide full blocks #17197

Merged
merged 2 commits into from Aug 27, 2019

Conversation

@kjellr
Copy link
Contributor

commented Aug 26, 2019

This cleans up a few bugs all related to the toolbar position for wide and full blocks.

  1. Eliminates the need for a horizontal scrollbar on screens between 600-782px wide.
  2. Corrects toolbar position on screens above 1080px.
  3. Corrects toolbar controls position on screens between 480-600px wide.

1. Eliminate the need for a horizontal scrollbar on screens between 600-782px wide

Closes #17137

This PR temoves the scrollbar that was showing up when selecting a full-width block in between the 600-782px breakpoints. We needed the toolbar's width value to negate its child's position value as defined here.

Before (Horizontal scrollbar is visible):

Screen Shot 2019-08-26 at 9 37 05 AM

After (Horizontal scrollbar is not visible):

Screen Shot 2019-08-26 at 9 34 34 AM


2. Correct toolbar position on screens above 1080px

This PR also adds new breakpoint overriding that rule, ensuring that the wide/full toolbars line up with those for standard blocks on large screens:

Before:

before

After:

after


Correct toolbar controls position on screens between 480-600px wide

Before:

Screen Shot 2019-08-26 at 9 46 00 AM

After:

Screen Shot 2019-08-26 at 9 45 43 AM

@kjellr kjellr added the [Type] Bug label Aug 26, 2019
@kjellr kjellr requested review from jasmussen and jorgefilipecosta Aug 26, 2019
@kjellr kjellr requested review from ellatrix, talldan and youknowriad as code owners Aug 26, 2019
@kjellr kjellr self-assigned this Aug 26, 2019
Copy link
Contributor

left a comment

Works great for me. Thanks for such a thorough and good fix.

Incidentally it also works well in IE11, better than non wide blocks actually. I'm going to look at IE11 today so I may take a look at your magic here :)

@kjellr kjellr merged commit d29289c into master Aug 27, 2019
2 checks passed
2 checks passed
pull-request-automation
Details
Travis CI - Pull Request Build Passed
Details
@kjellr kjellr deleted the update/toolbar-position-for-wide-full-blocks branch Aug 27, 2019
@jasmussen

This comment has been minimized.

Copy link
Contributor

commented Aug 28, 2019

I don't think the following is a regression from this PR, and if so I apologize for not catching it. But I'm seeing a horizontal scrollbar on a fullwide gallery. Any idea what this might be?

@kjellr

This comment has been minimized.

Copy link
Contributor Author

commented Aug 28, 2019

Argh. It's definitely not from this PR at least. I reverted this change locally and I'm still seeing the scrollbar.

It looks like there's 4px of extra space somewhere... if I set wp-block-gallery to be calc(100% - 4px), the scrollbar goes away (but things appear slightly off. 😕). We'll need to do some more digging to figure out exactly where that's coming from.

@kjellr

This comment has been minimized.

Copy link
Contributor Author

commented Aug 28, 2019

(I've created #17233 to continue tracking the issue.)

@jasmussen

This comment has been minimized.

Copy link
Contributor

commented Aug 28, 2019

♥️

@gziolo gziolo added this to the Gutenberg 6.5 milestone Aug 30, 2019
dd32 pushed a commit to dd32/gutenberg that referenced this pull request Sep 27, 2019
* Toolbar position cleanup.

* Cleanup wide/full toolbar position between 480 and 600px.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.