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

Cleanup CSS variables #20529

Merged
merged 2 commits into from Mar 2, 2020
Merged

Cleanup CSS variables #20529

merged 2 commits into from Mar 2, 2020

Conversation

@youknowriad
Copy link
Contributor

youknowriad commented Feb 28, 2020

Some cleaning to our CSS variables.

  • Rename icon-button-size to just button-size
  • Remove deprecated grid units
  • A few other tweaks
@github-actions

This comment has been minimized.

Copy link

github-actions bot commented Feb 28, 2020

Size Change: -11 B (0%)

Total Size: 866 kB

Filename Size Change
build/block-editor/style-rtl.css 10.5 kB -6 B (0%)
build/block-editor/style.css 10.5 kB -5 B (0%)
ℹ️ View Unchanged
Filename Size Change
build/a11y/index.js 1.01 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-library/editor-rtl.css 7.66 kB 0 B
build/block-library/editor.css 7.66 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.5 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 191 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.54 kB 0 B
build/edit-post/style.css 8.54 kB 0 B
build/edit-site/index.js 4.63 kB 0 B
build/edit-site/style-rtl.css 2.51 kB 0 B
build/edit-site/style.css 2.51 kB 0 B
build/edit-widgets/index.js 4.42 kB 0 B
build/edit-widgets/style-rtl.css 2.59 kB 0 B
build/edit-widgets/style.css 2.58 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.48 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 780 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

@@ -1,11 +1,11 @@
.block-editor-color-gradient-control {
&__color-indicator {
margin-bottom: $grid-size;
margin-bottom: $grid-unit;

This comment has been minimized.

Copy link
@jasmussen

jasmussen Feb 28, 2020

Contributor

I don't think we should ever actually use $grid-unit, it should always be one of the numbered ones. I.e. in this case, it should be $grid-unit-10.

In that vein, $grid-unit is intended only to seed all the numbered variables. Should it have a different name to reflect that?

This comment has been minimized.

Copy link
@youknowriad

youknowriad Feb 28, 2020

Author Contributor

I updated but why not just remove the variable in that case and use $grid-unit-10 everywhere.

This comment has been minimized.

Copy link
@jasmussen

jasmussen Feb 28, 2020

Contributor

Oh I guess that could work? Seems almost cyclical 😅

The alternative would be to spell out 8px for each variable.

@@ -48,7 +48,6 @@

// Ensure that the height of the first appender, and the one between blocks, is the same as text.
.block-editor-default-block-appender__content {
min-height: $empty-paragraph-height / 2;

This comment has been minimized.

Copy link
@jasmussen

jasmussen Feb 28, 2020

Contributor

I haven't had time to test this yet, are you sure there aren't any negataive consequences of removing this? I'm sure we added it for a reason, back in the day.

This comment has been minimized.

Copy link
@youknowriad

youknowriad Feb 28, 2020

Author Contributor

The reason we added it is to make sure the appender with or without paragraph has the same height. It's broken with or without this style now.

This comment has been minimized.

Copy link
@jasmussen

jasmussen Feb 28, 2020

Contributor

Not sure I follow but I'll check when I have a moment.

@jasmussen

This comment has been minimized.

Copy link
Contributor

jasmussen commented Feb 28, 2020

In general this is a really nice bit of cleanup, thank you. I promise I'll get to the color variable cleanup as well, perhaps in my color PR. I left two comments to address.

@youknowriad youknowriad force-pushed the update/css-variables branch from 80cc8c1 to 30c78f7 Feb 28, 2020
@youknowriad youknowriad merged commit 7866ae1 into master Mar 2, 2020
3 checks passed
3 checks passed
build
Details
pull-request-automation
Details
Travis CI - Pull Request Build Passed
Details
@youknowriad youknowriad deleted the update/css-variables branch Mar 2, 2020
@github-actions github-actions bot added this to the Gutenberg 7.7 milestone Mar 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.