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

Remove !important override from Buttons block #20433

Merged
merged 1 commit into from
Feb 25, 2020

Conversation

marekhrabe
Copy link
Contributor

Description

Adding the Buttons block in #17352 has broken previews because it adds !important to setting the appender to display it as an inline block. Previews try their best to hide UI like this but adding the !important has prevented that from working.

Is it necessary? I have briefly tested the Buttons block without !important and there seems to be no obvious glitch happening. However, as the usage of the !important didn't seem to have been commented, I'm not sure what to look for and where it potentially breaks.

cc: @jorgefilipecosta as the original author

How has this been tested?

  • Insert Buttons block
  • Test its basic functionality
  • Make sure appender appears on the same line with buttons (if space permits)

Screenshots

Screenshot 2020-02-25 at 12 57 36

Types of changes

Bug fix (non-breaking change which fixes an issue)

Checklist:

  • My code follows the WordPress code style.

@github-actions
Copy link

Size Change: -7 B (0%)

Total Size: 865 kB

Filename Size Change
build/block-library/editor-rtl.css 7.67 kB -4 B (0%)
build/block-library/editor.css 7.67 kB -3 B (0%)
ℹ️ View Unchanged
Filename Size Change
build/a11y/index.js 1 kB 0 B
build/annotations/index.js 3.43 kB 0 B
build/api-fetch/index.js 3.39 kB 0 B
build/autop/index.js 2.58 kB 0 B
build/blob/index.js 620 B 0 B
build/block-directory/index.js 6.02 kB 0 B
build/block-directory/style-rtl.css 760 B 0 B
build/block-directory/style.css 760 B 0 B
build/block-editor/index.js 104 kB 0 B
build/block-editor/style-rtl.css 10.3 kB 0 B
build/block-editor/style.css 10.3 kB 0 B
build/block-library/index.js 116 kB 0 B
build/block-library/style-rtl.css 7.49 kB 0 B
build/block-library/style.css 7.49 kB 0 B
build/block-library/theme-rtl.css 669 B 0 B
build/block-library/theme.css 671 B 0 B
build/block-serialization-default-parser/index.js 1.65 kB 0 B
build/block-serialization-spec-parser/index.js 3.1 kB 0 B
build/blocks/index.js 57.6 kB 0 B
build/components/index.js 190 kB 0 B
build/components/style-rtl.css 15.5 kB 0 B
build/components/style.css 15.5 kB 0 B
build/compose/index.js 5.76 kB 0 B
build/core-data/index.js 10.5 kB 0 B
build/data-controls/index.js 1.03 kB 0 B
build/data/index.js 8.22 kB 0 B
build/date/index.js 5.37 kB 0 B
build/deprecated/index.js 772 B 0 B
build/dom-ready/index.js 568 B 0 B
build/dom/index.js 3.06 kB 0 B
build/edit-post/index.js 90.9 kB 0 B
build/edit-post/style-rtl.css 8.68 kB 0 B
build/edit-post/style.css 8.67 kB 0 B
build/edit-site/index.js 4.59 kB 0 B
build/edit-site/style-rtl.css 2.77 kB 0 B
build/edit-site/style.css 2.76 kB 0 B
build/edit-widgets/index.js 4.36 kB 0 B
build/edit-widgets/style-rtl.css 2.8 kB 0 B
build/edit-widgets/style.css 2.79 kB 0 B
build/editor/editor-styles-rtl.css 325 B 0 B
build/editor/editor-styles.css 327 B 0 B
build/editor/index.js 44.6 kB 0 B
build/editor/style-rtl.css 4.01 kB 0 B
build/editor/style.css 4 kB 0 B
build/element/index.js 4.45 kB 0 B
build/escape-html/index.js 733 B 0 B
build/format-library/index.js 7.6 kB 0 B
build/format-library/style-rtl.css 502 B 0 B
build/format-library/style.css 502 B 0 B
build/hooks/index.js 1.92 kB 0 B
build/html-entities/index.js 622 B 0 B
build/i18n/index.js 3.45 kB 0 B
build/is-shallow-equal/index.js 710 B 0 B
build/keyboard-shortcuts/index.js 2.3 kB 0 B
build/keycodes/index.js 1.68 kB 0 B
build/list-reusable-blocks/index.js 2.99 kB 0 B
build/list-reusable-blocks/style-rtl.css 226 B 0 B
build/list-reusable-blocks/style.css 226 B 0 B
build/media-utils/index.js 4.85 kB 0 B
build/notices/index.js 1.57 kB 0 B
build/nux/index.js 3.02 kB 0 B
build/nux/style-rtl.css 616 B 0 B
build/nux/style.css 613 B 0 B
build/plugins/index.js 2.54 kB 0 B
build/primitives/index.js 1.49 kB 0 B
build/priority-queue/index.js 879 B 0 B
build/redux-routine/index.js 2.84 kB 0 B
build/rich-text/index.js 14.3 kB 0 B
build/server-side-render/index.js 2.54 kB 0 B
build/shortcode/index.js 1.7 kB 0 B
build/token-list/index.js 1.27 kB 0 B
build/url/index.js 4 kB 0 B
build/viewport/index.js 1.61 kB 0 B
build/warning/index.js 1.14 kB 0 B
build/wordcount/index.js 1.18 kB 0 B

compressed-size-action

@getdave
Copy link
Contributor

getdave commented Feb 25, 2020

This looks like the !important could easily have some knock-on effects if anyone was relying on this selector to hide the appenders.

Unless @jorgefilipecosta knows of anything, then it seems like removing the !important would be a good way to go to avoid future CSS specificity problems.

@retrofox
Copy link
Contributor

Yeah, just wondering the cause of adding !important there. If the block still works as expected despite removing the !important rule I don't see why we should keep it. Maybe are we missing something?

@jorgefilipecosta jorgefilipecosta added the Backport to WP 6.7 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta label Feb 25, 2020
Copy link
Member

@jorgefilipecosta jorgefilipecosta left a comment

Choose a reason for hiding this comment

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

LGTM 👍 I tried this PR on the trunk branch and this still works as they should. I think important was added by mistake.

@marekhrabe
Copy link
Contributor Author

The Travis CI errors with this message:

Error: sandbox not initialized

It doesn't look related in any way but it's failing quite consistently for me. Can I still merge or should I retry until I might have a good luck?

@jorgefilipecosta jorgefilipecosta added this to the Gutenberg 7.6 milestone Feb 25, 2020
@jorgefilipecosta jorgefilipecosta merged commit 7c21d2c into master Feb 25, 2020
@jorgefilipecosta
Copy link
Member

Hi @marekhrabe, I merged the change.

@aduth aduth deleted the remove/important-css-from-buttons-appender branch February 25, 2020 20:56
@jorgefilipecosta jorgefilipecosta removed the Backport to WP 6.7 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta label Mar 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Buttons Affects the Buttons Block
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants