Cover: Add custom unit support for height controls #20888

ItsJonQ · 2020-03-13T20:21:09Z

Description

This update adds a new (experimental) UnitControl that enables values beyond pixels! For Cover block, this control unlocks the ability to set px, em, rem, vw and vh values.

(Note: Although % is supported, it's omitted from the Cover block control as % height does not render predictably).

How has this been tested?

New (experimental) UnitControl and NumberControl were tested in Storybook and Gutenberg
UnitControl x Cover block integration was developed and tested in local Gutenberg

Screenshots

The GIF above showcases the flow for changing values and unit values for the Cover block.

Types of changes

UnitControl (and NumberControl)

These new experimental components have been added to @wordpress/components. The UnitControl is the component that powers the input for setting the height for the Cover block.

Shift Increment

One of the features with this new control is to jump values when holding down shift while pressing up or down (default is by 10). This interaction is common for other Design applications like Figma, Sketch, Photoshop, etc...

Note: This feature can be customized and even disabled in the base component.

Switching Units

The original behaviour defaults to a min-height of 50px, but only if the user makes a change. This interaction is preserved in this update.

To improve the canvas rendering of Cover, switching units would "reset" the Cover to predetermined values for each unit. This is to prevent the Cover block's height from unexpectedly jumping to 3000px if they switch to a vh or vw unit.

Drag Resize Handling

Dragging to resize will "reset" the unit back to px. This interaction decision was made as it is very difficult to accurately translate pixel by pixel drag values to their respective vh or em values.

Simulated Height Rendering

Cover's canvas rendering is simulated as best as possible given the value and unit type. For example, vh and vw can be accurately rendered. Whereas we have to take the best guess for units like em or rem.

Tests

These updates have been tested locally in Gutenberg. Unit tests need to be written for the new UnitControl and NumberControl components.

Checklist:

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

Resolves:
#20567 (comment)

github-actions · 2020-03-13T20:25:48Z

Size Change: +1.99 kB (0%)

Total Size: 863 kB

Filename	Size	Change
`build/annotations/index.js`	3.44 kB	+1 B
`build/api-fetch/index.js`	3.39 kB	+1 B
`build/block-directory/index.js`	6.03 kB	+4 B (0%)
`build/block-editor/index.js`	102 kB	+136 B (0%)
`build/block-editor/style-rtl.css`	11 kB	+6 B (0%)
`build/block-editor/style.css`	11 kB	+6 B (0%)
`build/block-library/index.js`	110 kB	+196 B (0%)
`build/block-serialization-default-parser/index.js`	1.65 kB	+1 B
`build/blocks/index.js`	57.5 kB	+2 B (0%)
`build/components/index.js`	191 kB	+1.61 kB (0%)
`build/core-data/index.js`	10.6 kB	+1 B
`build/data-controls/index.js`	1.03 kB	-3 B (0%)
`build/data/index.js`	8.26 kB	+4 B (0%)
`build/date/index.js`	5.36 kB	-4 B (0%)
`build/edit-site/index.js`	8.63 kB	+3 B (0%)
`build/edit-widgets/index.js`	4.43 kB	-1 B
`build/editor/index.js`	42.8 kB	+11 B (0%)
`build/element/index.js`	4.44 kB	+2 B (0%)
`build/format-library/index.js`	6.95 kB	-2 B (0%)
`build/hooks/index.js`	1.92 kB	-2 B (0%)
`build/i18n/index.js`	3.57 kB	+1 B
`build/is-shallow-equal/index.js`	712 B	+2 B (0%)
`build/keyboard-shortcuts/index.js`	2.31 kB	+7 B (0%)
`build/keycodes/index.js`	1.7 kB	-2 B (0%)
`build/media-utils/index.js`	4.84 kB	+3 B (0%)
`build/notices/index.js`	1.57 kB	-2 B (0%)
`build/nux/index.js`	3.01 kB	+3 B (0%)
`build/plugins/index.js`	2.54 kB	+3 B (0%)
`build/primitives/index.js`	1.5 kB	-1 B
`build/redux-routine/index.js`	2.84 kB	+1 B
`build/rich-text/index.js`	14.5 kB	+2 B (0%)
`build/server-side-render/index.js`	2.55 kB	-3 B (0%)
`build/shortcode/index.js`	1.7 kB	+1 B
`build/url/index.js`	4.01 kB	+2 B (0%)
`build/warning/index.js`	1.14 kB	-1 B

ℹ️

View Unchanged

Filename	Size	Change
`build/a11y/index.js`	998 B	0 B
`build/autop/index.js`	2.58 kB	0 B
`build/blob/index.js`	620 B	0 B
`build/block-directory/style-rtl.css`	760 B	0 B
`build/block-directory/style.css`	760 B	0 B
`build/block-library/editor-rtl.css`	7.21 kB	0 B
`build/block-library/editor.css`	7.21 kB	0 B
`build/block-library/style-rtl.css`	7.5 kB	0 B
`build/block-library/style.css`	7.51 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-spec-parser/index.js`	3.1 kB	0 B
`build/components/style-rtl.css`	15.8 kB	0 B
`build/components/style.css`	15.7 kB	0 B
`build/compose/index.js`	6.2 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`	92.3 kB	0 B
`build/edit-post/style-rtl.css`	8.35 kB	0 B
`build/edit-post/style.css`	8.34 kB	0 B
`build/edit-site/style-rtl.css`	3.44 kB	0 B
`build/edit-site/style.css`	3.44 kB	0 B
`build/edit-widgets/style-rtl.css`	2.57 kB	0 B
`build/edit-widgets/style.css`	2.57 kB	0 B
`build/editor/editor-styles-rtl.css`	423 B	0 B
`build/editor/editor-styles.css`	426 B	0 B
`build/editor/style-rtl.css`	3.38 kB	0 B
`build/editor/style.css`	3.38 kB	0 B
`build/escape-html/index.js`	733 B	0 B
`build/format-library/style-rtl.css`	502 B	0 B
`build/format-library/style.css`	502 B	0 B
`build/html-entities/index.js`	622 B	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/nux/style-rtl.css`	616 B	0 B
`build/nux/style.css`	613 B	0 B
`build/priority-queue/index.js`	781 B	0 B
`build/token-list/index.js`	1.28 kB	0 B
`build/viewport/index.js`	1.61 kB	0 B
`build/wordcount/index.js`	1.18 kB	0 B

_{compressed-size-action}

mcsf · 2020-03-16T19:02:42Z

Hi! I may have missed a detail, but was wondering: what are some intended use cases for allowing the customisation of the unit set beyond px, em, rem, vw, vh?

mtias · 2020-03-16T19:22:05Z

lib/client-assets.php

+ * @return array Filtered editor settings.
+ */
+function gutenberg_extend_settings_custom_units( $settings ) {
+	$settings['__experimentalDisableCustomUnits'] = get_theme_support( 'disable-custom-units' );


Should this support a second argument where the theme can say which should be the unit shown / used? Like get_theme_support( 'disable-custom-units', 'rem' ) would force rem to be used in all unit-based selectors.

Good suggestion! Let me make that adjustment ✨

ItsJonQ · 2020-03-17T19:45:35Z

Hi! I may have missed a detail, but was wondering: what are some intended use cases for allowing the customisation of the unit set beyond px, em, rem, vw, vh?

@mcsf Haii!! There are many other units beyond the defaults I've set for the base UnitControl. My intention was to keep it open, which may allow blocks (core or 3rd party) to specify the units they wish to support. Some other custom units (that are slowly gaining popularity) are things like vmin or vmax.

Hope this makes sense 🙏

ItsJonQ · 2020-03-17T20:47:20Z

@mtias I just pushed an update! This adds support for additional parameters for add_theme_support( 'disable-custom-units').

For example:

// This will disable all custom units
add_theme_support( 'disable-custom-units' );

// This will enable only rem
add_theme_support( 'disable-custom-units', 'rem' );

// This will enable only rem and em
add_theme_support( 'disable-custom-units', 'rem', 'em' );

If, say, only rem and em are enabled, a control would look like this:

Lemme know how this API feels for you :)

In other news...

There may be race conditions where a block (by default) expects a px value. However, the moment the units are forced to using only rem, it may make the block confused.

We have a bit of this in the case of Cover. To test...

Add a Cover block
Save
Add add_theme_support( 'disable-custom-units', 'rem' );
Change Cover block's height

mtias · 2020-03-18T12:02:11Z

This looks good. Yes, there will be several things to tweak to account for units being a "thing" now across blocks, but we'll get there.

It might make sense to rename the theme support from disable-custom-units to experimental-custom-units initially in case we need to make tweaks before releasing.

ItsJonQ · 2020-03-19T15:51:25Z

It might make sense to rename the theme support from disable-custom-units to experimental-custom-units initially in case we need to make tweaks before releasing.

@mtias Done 👍

mcsf

I've left some notes, but other than that let's keep it moving!

mcsf · 2020-03-19T16:37:58Z

packages/block-library/src/cover/block.json

@@ -32,6 +32,9 @@
 		"minHeight": {
 			"type": "number"
 		},
+		"minHeightUnit": {
+			"type": "string"


I assume from this that it is preferred to let components like CoverHeightInput decide based on an undefined value, correct? Otherwise we might add a default value here.

mcsf · 2020-03-19T16:37:58Z

packages/block-library/src/cover/save.js

-	style.minHeight = minHeight || undefined;
+	style.minHeight = minHeightUnit
+		? `${ minHeight }${ minHeightUnit }`
+		: minHeight || undefined;


I for one had to go check the operator precedence table for logical OR and ternary conditional to make sure I was reading this correctly. :) I recommend parentheses in this situations.

mcsf · 2020-03-19T16:37:58Z

packages/components/src/utils/colors-values.js

 export const BASE = {
 	black: '#000',
 	white: '#fff',
 };

+export const G2 = {


Maybe add TODO-type comment explaining what G2 means (succinctly, focused on why this object is separate from the other colors) and with a note on when it would be a good time to consolidate all of this. Otherwise other devs in a few months won't have a clue of what this means. :)

ItsJonQ · 2020-03-19T16:55:41Z

@mcsf Thank you so much for your feedback 🙏 ! Great suggestions! I just pushed them up

ItsJonQ · 2020-03-20T14:28:49Z

@mcsf Hmm! It looks like adding the default px value for minHeightUnit breaks a couple of the fixture based tests 😅

I'm not sure how to fix it. It looks like the fixture (.json) files are autogenerated. I tried running npm run fixtures:generate, but it doesn't look like that's helping.

🤔

youknowriad · 2020-03-20T14:31:26Z

It also means you need a deprecated version for the block, otherwise, it's going to invalidate existing blocks.

aduth · 2020-03-20T14:48:33Z

packages/components/src/utils/colors-values.js

 export const COLORS = {
 	...BASE,
-	darkGray: DARK_GRAY,
+	darkGray: merge( DARK_GRAY, G2.darkGray ),


This is mutating DARK_GRAY, such that every reference to DARK_GRAY would subsequently contain properties of G2.darkGray. I don't think that's the intention?

Suggested change

darkGray: merge( DARK_GRAY, G2.darkGray ),

darkGray: merge( {}, DARK_GRAY, G2.darkGray ),

A few other instances in this file as well.

Oh! I didn't know it mutates. Thank you! Will fix those up

ItsJonQ · 2020-03-23T14:09:52Z

Rebased with latest master

ItsJonQ · 2020-03-24T14:01:33Z

It also means you need a deprecated version for the block, otherwise, it's going to invalidate existing blocks.

@youknowriad Haiii! I'm following up on this PR :).

I'm not sure how to proceed with deprecation. For this particular change, would adding a new minHeightUnit qualify for us to deprecate the previous version of Cover?

Thank you!

The block-editor variant of UnitControl has add_theme_support built in

…in inline style.

…ble-custom-units' )`

ItsJonQ · 2020-03-30T17:58:51Z

Yay! Looks like Travis green and happy!
Thanks for the reviews, thoughts, and feedback everyone!

Excited for this one to land.

Merging it up!

oandregal · 2020-04-13T17:18:52Z

packages/components/src/unit-control/styles/unit-control-styles.js

+
+const baseUnitLabelStyles = ( props ) => {
+	return css`
+		appearance: none;


I'm testing this on Firefox 75 in a Linux OS and the input control in Firefox includes the stepper arrows buttons:

while in Chrome it doesn't:

For what is worth, I've tried with -moz-appearance: textfield and, as far as my testing went, it worked as expected (arrow up/down with the keyboard still increases/decreases the number) and the steppers are not shown.

cc @ItsJonQ @mcsf

I'm testing this on Firefox 75 in a Linux OS

Firefox (76) on macOS is also affected.

Potential fix at #21571

mems · 2020-11-11T14:46:28Z

For people like me who don't know why this option is not available in the editor of cover block and find that PR, it's because the theme didn't enable custom units:

add_theme_support( 'custom-units' ); // this allow the post author to select other CSS units than "px"

ItsJonQ added the [Block] Cover label Mar 13, 2020

ItsJonQ requested a review from mtias Mar 13, 2020

ItsJonQ requested review from ajitbohra, chrisvanpatten, jorgefilipecosta, Soean, talldan, TimothyBJacobs and youknowriad as code owners Mar 13, 2020

ItsJonQ self-assigned this Mar 13, 2020

ItsJonQ requested a review from ellatrix as a code owner Mar 14, 2020

mtias reviewed Mar 16, 2020

View changes

ItsJonQ force-pushed the try/cover-height-custom-units branch from 4eecd7b to 7feead3 Mar 18, 2020

ItsJonQ mentioned this pull request Mar 19, 2020

Paragraph: Add LineHeightControl + Attribute #20775

Merged

5 of 6 tasks

mcsf approved these changes Mar 19, 2020

View changes

aduth reviewed Mar 20, 2020

View changes

ItsJonQ force-pushed the try/cover-height-custom-units branch from 0a99fa5 to dd9e79d Mar 23, 2020

ItsJonQ force-pushed the try/cover-height-custom-units branch from dd9e79d to d1469b6 Mar 24, 2020

ItsJonQ mentioned this pull request Mar 25, 2020

Adds dimension controls to Group Block #16730

Closed

3 of 11 tasks

ItsJonQ added 19 commits Mar 13, 2020

Add add_theme_support for custom units

125405f

Adjust styles for UnitLabel

c411186

Add unit tests for NumberControl. Update snapshot for UnitControl.

fc8e56d

Abstract UnitControl to block-editor component

34a4bab

The block-editor variant of UnitControl has add_theme_support built in

Refactor to remove getComputedSize. Set unitized height directly with…

1dcc082

…in inline style.

Improve reset values for UnitControl

50ed451

Enable support for secondary parameters for `add_theme_support( 'disa…

8b43e60

…ble-custom-units' )`

Add comments for unit filtering

7691e30

Rename disable-custom-units to experimental-custom-units

a3998f3

Provide return description for filterUnitsWithSettings util

2b6ea59

Improve logic clarity of Cover minHeight style

34bafa0

Add dev notes for G2 color map

23e79b7

Default minHeightUnit of Cover to px

1b3a583

Adjust colors to deep merge without mutation

0a65b6d

Adjust label of Cover height control

8eaf5a6

Remove unnecessary test comments from number-control test

455cb04

Add tests for UnitControl

94b3a85

Update Storybook snapshots

52882de

Remove default px value for minHeightUnit in Cover

95a7024

ItsJonQ force-pushed the try/cover-height-custom-units branch from 6d1f7d8 to 95a7024 Mar 30, 2020

ItsJonQ added 2 commits Mar 30, 2020

Update PHP comments for line height and unit filters

fab4f91

Fix PHP lint issues

0d8f031

ItsJonQ merged commit 328655b into master Mar 30, 2020
3 checks passed

3 checks passed

build
Details

pull-request-automation
Details

Travis CI - Pull Request Build Passed
Details

ItsJonQ deleted the try/cover-height-custom-units branch Mar 30, 2020

github-actions bot added this to the Gutenberg 7.9 milestone Mar 30, 2020

oandregal reviewed Apr 13, 2020

View changes

ellatrix mentioned this pull request Jun 16, 2020

WP 5.5: merge all PHP changes #23210

Closed

10 of 12 tasks

jasmussen mentioned this pull request Feb 12, 2021

Custom unit support should be opt-out or default #28963

Closed

WordPress / gutenberg

Cover: Add custom unit support for height controls #20888

Cover: Add custom unit support for height controls #20888

ItsJonQ commented Mar 13, 2020 •

edited

github-actions bot commented Mar 13, 2020 •

edited

mcsf commented Mar 16, 2020

This comment has been minimized.

This comment has been minimized.

ItsJonQ commented Mar 17, 2020

ItsJonQ commented Mar 17, 2020

mtias commented Mar 18, 2020

ItsJonQ commented Mar 19, 2020

mcsf left a comment

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

ItsJonQ commented Mar 19, 2020

ItsJonQ commented Mar 20, 2020

youknowriad commented Mar 20, 2020

This comment has been minimized.

This comment has been minimized.

ItsJonQ commented Mar 23, 2020

ItsJonQ commented Mar 24, 2020

ItsJonQ commented Mar 30, 2020

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

mems commented Nov 11, 2020

	darkGray: merge( DARK_GRAY, G2.darkGray ),
	darkGray: merge( {}, DARK_GRAY, G2.darkGray ),

WordPress / gutenberg

Cover: Add custom unit support for height controls #20888

Cover: Add custom unit support for height controls #20888

Conversation

ItsJonQ commented Mar 13, 2020 • edited

Description

How has this been tested?

Screenshots

Types of changes

UnitControl (and NumberControl)

Shift Increment

Switching Units

Drag Resize Handling

Simulated Height Rendering

Tests

Checklist:

github-actions bot commented Mar 13, 2020 • edited

mcsf commented Mar 16, 2020

This comment has been minimized.

mtias Mar 16, 2020 Contributor

This comment has been minimized.

ItsJonQ Mar 17, 2020 Author Contributor

ItsJonQ commented Mar 17, 2020

ItsJonQ commented Mar 17, 2020

mtias commented Mar 18, 2020

ItsJonQ commented Mar 19, 2020

mcsf left a comment

This comment has been minimized.

mcsf Mar 19, 2020 Contributor

This comment has been minimized.

mcsf Mar 19, 2020 Contributor

This comment has been minimized.

mcsf Mar 19, 2020 Contributor

ItsJonQ commented Mar 19, 2020

ItsJonQ commented Mar 20, 2020

youknowriad commented Mar 20, 2020

This comment has been minimized.

aduth Mar 20, 2020 Member

This comment has been minimized.

ItsJonQ Mar 20, 2020 Author Contributor

ItsJonQ commented Mar 23, 2020

ItsJonQ commented Mar 24, 2020

ItsJonQ commented Mar 30, 2020

This comment has been minimized.

oandregal Apr 13, 2020 Member

This comment has been minimized.

mcsf Apr 13, 2020 Contributor

This comment has been minimized.

oandregal Apr 14, 2020 Member

mems commented Nov 11, 2020

ItsJonQ commented Mar 13, 2020 •

edited

github-actions bot commented Mar 13, 2020 •

edited

mtias Mar 16, 2020
Contributor

ItsJonQ Mar 17, 2020
Author Contributor

mcsf Mar 19, 2020
Contributor

mcsf Mar 19, 2020
Contributor

mcsf Mar 19, 2020
Contributor

aduth Mar 20, 2020
Member

ItsJonQ Mar 20, 2020
Author Contributor

oandregal Apr 13, 2020
Member

mcsf Apr 13, 2020
Contributor

oandregal Apr 14, 2020
Member