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

Fix gallery block toolbar overlap #1910

Merged
merged 2 commits into from
Jun 2, 2021
Merged

Fix gallery block toolbar overlap #1910

merged 2 commits into from
Jun 2, 2021

Conversation

ehamwey
Copy link
Contributor

@ehamwey ehamwey commented May 27, 2021

Description

Gallery block toolbars were overlapping under the site header due to a missing class, this provides a workaround by always applying that css rule to each block toolbar.

Additional findings:

  • With the coblocks plugin disabled, this behavior does not occur
  • With the coblocks plugin enabled, all gallery toolbars (core and coblocks) are missing the components-accessible-toolbar class. This prevents the core css rule display: inline-flex from being applied, which fixes the overlap issue.

Screenshots

Before:
image
After:
Screen Shot 2021-05-27 at 6 03 59 PM

Types of changes

CSS rule update

How has this been tested?

Locally, in-editor

Checklist:

  • My code is tested
  • My code follows accessibility standards
  • My code has proper inline documentation
  • I've included any necessary tests
  • I've included developer documentation
  • I've added proper labels to this pull request

Gallery block toolbars were overlapping under the site header due to a missing class header, this provides a workaround.
@ehamwey ehamwey added [Type] Bug Something that is not working as expected [Status] Needs Review Tracking pull requests that need another set of eyes labels May 27, 2021
@ehamwey ehamwey requested a review from AnthonyLedesma May 27, 2021 22:06
@ehamwey ehamwey requested review from jasonlemay and removed request for jasonlemay May 27, 2021 22:09
Copy link
Member

@olafleur olafleur left a comment

Choose a reason for hiding this comment

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

I was able to reproduce the fix. Good job! :)

Copy link
Member

@AnthonyLedesma AnthonyLedesma left a comment

Choose a reason for hiding this comment

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

Looks Good To Merge.

This issue was a bit strange in that the Toolbar component seems to have this missing class/style behavior when we are extending the existing toolbar on blocks with any custom controls.

This here is the spot in Gutenberg where the Toolbar is returned early without the 'components-accessible-toolbar' class.
https://github.com/WordPress/gutenberg/blob/7db6ba5cec54c64418600aa8f0e98088a0ba9cf4/packages/components/src/toolbar/index.js#L29-L42

Though the short-circuit is related to the toolbar label, this behavior exists when appending labels on all toolbars within CoBlocks.

@ehamwey ehamwey merged commit bdcdb73 into master Jun 2, 2021
@ehamwey ehamwey deleted the fix/toolbar-overlap branch June 2, 2021 14:54
@Mamaduka
Copy link

Mamaduka commented Jun 3, 2021

The issue is fixed in the Gutenberg plugin as well - WordPress/gutenberg#32424.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Status] Needs Review Tracking pull requests that need another set of eyes [Type] Bug Something that is not working as expected
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants