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

Theme Support: Enable custom units by default #24873

Closed
wants to merge 1 commit into from

Conversation

ItsJonQ
Copy link

@ItsJonQ ItsJonQ commented Aug 27, 2020

This update enables custom units by default. Custom units can be disabled using add_theme_support( 'custom-unit' ) and setting it to false, like so:

add_theme_support( 'custom-unit', false )

A quick enhancement for this: #23964 (comment)

Thinking behind this approach

Since the experimental flag was removed from custom-unit, I felt hesitant changing that filter key. Unlike some of the other design tool filters, this one acts as more than just a boolean value.

For instance, themers can filter certain units by doing the following:

add_theme_support( 'custom-unit', 'em, 'px'  );

(The above would use only em and px).

Instead of adding something like add_theme_support( 'disable-custom-unit' )... I preserved the custom-unit theme support key, but adjusted the way it could be used to disable + filter units.

Default: Custom units on

// This does nothing now.
add_theme_support( 'custom-unit' );

Disable: Custom units off

add_theme_support( 'custom-unit', false );

Filtering: Custom units on, but filtered

add_theme_support( 'custom-unit', 'em', 'rem' );

Working with (global) custom units

At the moment, Cover appears to be the only core block that uses UnitControl, specifically for height and padding.

For the units to respect the add_theme_support, the component (or hook) must come from:

packages/block-editor/src/components/unit-control/index.js

This will have the theme-configured units pre-bound:

import { __experimentalUnitControl as UnitControl } from '@wordpress/block-editor';

const Example = () => <UnitControl />

When working within block-editor, you can use the hook to receive pre-filtered units:

import { useCustomUnits } from '../components/unit-control';

const Example = () => {
  const units = useCustomUnits();
  return <UnitControl units={units}  />
}

Working with UnitControl

The recommended implementation of <UnitControl /> would be this setup:

// myValue = 45%
<UnitControl value={myValue} onChange={handleOnChange} isPressEnterToChange />

The unit value (string) contains both the value and the unit. isPressEnterToChange is important as it allows users to type in custom values, e.g. 12345px. The change only applies on ENTER press (or blur).

This is how it's setup with Cover's padding controls (via BoxControl).

An older (initial) implementation looked like this (Cover's height controls):

<UnitControl value={myValue} unit={myUnit} onChange={handleOnChange} onUnitChange={handleOnUnitChange}  />

Notice how the value and unit is tracked separately. Also, there isn't isPressEnterToChange, therefore the changes are applied immediately.

Adding custom units

If you need to add custom units to your control (e.g. vmin, %, etc...), you can do so like this:

const customUnits = [
	{ value: 'px', label: 'px', default: 430 },
	{ value: '%', label: '%', default: 20 },
	{ value: 'rem', label: 'rem', default: 20 },
];

<UnitControl units={customUnits} />

This will allow you to adjust unit values based on different design contexts.

An example of this can be seen in Cover:
https://github.com/WordPress/gutenberg/blob/master/packages/block-library/src/cover/shared.js#L21

How has this been tested?

  • Tested locally in Gutenberg
  • Add Cover block
  • Ensure that the Height control has custom units (presented in a dropdown)

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • [n/a] My code follows the accessibility standards.
  • [n/a] My code has proper inline documentation.
  • I've included developer documentation if appropriate.
  • [n/a] I've updated all React Native files affected by any refactorings/renamings in this PR.

@ItsJonQ ItsJonQ added Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json [Feature] Design Tools Tools that impact the appearance of blocks both to expand the number of tools and improve the experi labels Aug 27, 2020
@ItsJonQ ItsJonQ requested review from mtias and apeatling August 27, 2020 19:07
@ItsJonQ ItsJonQ self-assigned this Aug 27, 2020
@github-actions
Copy link

Size Change: +16 B (0%)

Total Size: 1.17 MB

Filename Size Change
build/block-editor/index.js 126 kB +16 B (0%)
ℹ️ View Unchanged
Filename Size Change
build/a11y/index.js 1.14 kB 0 B
build/annotations/index.js 3.67 kB 0 B
build/api-fetch/index.js 3.44 kB 0 B
build/autop/index.js 2.82 kB 0 B
build/blob/index.js 620 B 0 B
build/block-directory/index.js 7.99 kB 0 B
build/block-directory/style-rtl.css 953 B 0 B
build/block-directory/style.css 952 B 0 B
build/block-editor/style-rtl.css 10.8 kB 0 B
build/block-editor/style.css 10.8 kB 0 B
build/block-library/editor-rtl.css 8.52 kB 0 B
build/block-library/editor.css 8.52 kB 0 B
build/block-library/index.js 136 kB 0 B
build/block-library/style-rtl.css 7.45 kB 0 B
build/block-library/style.css 7.46 kB 0 B
build/block-library/theme-rtl.css 729 B 0 B
build/block-library/theme.css 730 B 0 B
build/block-serialization-default-parser/index.js 1.88 kB 0 B
build/block-serialization-spec-parser/index.js 3.1 kB 0 B
build/blocks/index.js 47.7 kB 0 B
build/components/index.js 200 kB 0 B
build/components/style-rtl.css 15.7 kB 0 B
build/components/style.css 15.7 kB 0 B
build/compose/index.js 9.67 kB 0 B
build/core-data/index.js 12.3 kB 0 B
build/data-controls/index.js 1.29 kB 0 B
build/data/index.js 8.55 kB 0 B
build/date/index.js 5.38 kB 0 B
build/deprecated/index.js 772 B 0 B
build/dom-ready/index.js 568 B 0 B
build/dom/index.js 4.48 kB 0 B
build/edit-navigation/index.js 11.7 kB 0 B
build/edit-navigation/style-rtl.css 1.16 kB 0 B
build/edit-navigation/style.css 1.16 kB 0 B
build/edit-post/index.js 304 kB 0 B
build/edit-post/style-rtl.css 5.61 kB 0 B
build/edit-post/style.css 5.61 kB 0 B
build/edit-site/index.js 17 kB 0 B
build/edit-site/style-rtl.css 3.06 kB 0 B
build/edit-site/style.css 3.06 kB 0 B
build/edit-widgets/index.js 11.9 kB 0 B
build/edit-widgets/style-rtl.css 2.45 kB 0 B
build/edit-widgets/style.css 2.45 kB 0 B
build/editor/editor-styles-rtl.css 537 B 0 B
build/editor/editor-styles.css 539 B 0 B
build/editor/index.js 45.3 kB 0 B
build/editor/style-rtl.css 3.8 kB 0 B
build/editor/style.css 3.79 kB 0 B
build/element/index.js 4.65 kB 0 B
build/escape-html/index.js 733 B 0 B
build/format-library/index.js 7.71 kB 0 B
build/format-library/style-rtl.css 547 B 0 B
build/format-library/style.css 548 B 0 B
build/hooks/index.js 2.13 kB 0 B
build/html-entities/index.js 621 B 0 B
build/i18n/index.js 3.56 kB 0 B
build/is-shallow-equal/index.js 710 B 0 B
build/keyboard-shortcuts/index.js 2.52 kB 0 B
build/keycodes/index.js 1.94 kB 0 B
build/list-reusable-blocks/index.js 3.12 kB 0 B
build/list-reusable-blocks/style-rtl.css 476 B 0 B
build/list-reusable-blocks/style.css 476 B 0 B
build/media-utils/index.js 5.32 kB 0 B
build/notices/index.js 1.79 kB 0 B
build/nux/index.js 3.4 kB 0 B
build/nux/style-rtl.css 671 B 0 B
build/nux/style.css 668 B 0 B
build/plugins/index.js 2.56 kB 0 B
build/primitives/index.js 1.41 kB 0 B
build/priority-queue/index.js 789 B 0 B
build/redux-routine/index.js 2.85 kB 0 B
build/rich-text/index.js 13.9 kB 0 B
build/server-side-render/index.js 2.77 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.06 kB 0 B
build/viewport/index.js 1.85 kB 0 B
build/warning/index.js 1.14 kB 0 B
build/wordcount/index.js 1.17 kB 0 B

compressed-size-action


```php
add_theme_support( 'custom-units', 'rem', 'em' );
add_theme_support( 'custom-units', false );
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should change this to remove_theme_support( 'custom-units' ); and add a add_theme_support( true ) call to Core.

Copy link
Author

Choose a reason for hiding this comment

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

Cool! That API makes sense to me. I've never worked with remove_theme_support before.. Is there an example I can follow to do that? :D

@aristath aristath added the [Feature] Themes Questions or issues with incorporating or styling blocks in a theme. label Aug 28, 2020
@mtias
Copy link
Member

mtias commented Aug 28, 2020

Shouldn't the specific units be a single array argument instead of spread arguments?

@ItsJonQ
Copy link
Author

ItsJonQ commented Aug 31, 2020

Shouldn't the specific units be a single array argument instead of spread arguments?

@mtias I'd lean on the opinions of others on this one!

custom-units was the first filter I helped create for Gutenberg with multiple arguments. I'm not sure what the ideal/most intuitive API would be. I'm happy to help change it to that though.

@youknowriad
Copy link
Contributor

Shouldn't the specific units be a single array argument instead of spread arguments?

I'd keep it as is just because it's already on WP 5.5. Ultimately, all these options will be configured using theme.json

@mtias
Copy link
Member

mtias commented Aug 31, 2020

Sounds good

@ItsJonQ
Copy link
Author

ItsJonQ commented Sep 15, 2020

Closing, if favour of:
#25217

@ItsJonQ ItsJonQ closed this Sep 15, 2020
@youknowriad youknowriad deleted the update/custom-unit-enabled-by-default branch September 15, 2020 15:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Design Tools Tools that impact the appearance of blocks both to expand the number of tools and improve the experi [Feature] Themes Questions or issues with incorporating or styling blocks in a theme. Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants