Polish the checkbox, update switch documentation (#8588)

* Polish the checkbox, update switch documentation The switch has a long history. Before I go into details, here's what this PR does: - It replaces Pending Review, Stick to Front Page, Allow Comments, and Allow Pingbacks from using Switches to using Checkboxes. - It tweaks the design of the checkbox to have a significantly more visible checked state, and a new more visual focus state. - It tweaks the documentation for when to use a switch instead of a checkbox. The Switch is born from mobile, which is increasingly the device of choice for people and long term might become the only choice. It's bigger and more tappable, and implies by its nature that it's a boolean choice. It also has qualities that the checkbox does not embody: - Like its wall lightswitch counterpart, it’s something you flip and the effect is instant. There used to be darkness, now there’s light. - It has only a single purpose, and that purpose is boolean. For example you wouldn’t use switches to select multiple categories. However Allow Comments and the three other aforementioned toggles that use switches in master, do not have an instant effect, they require you to hit "Save" before the toggle option is stored. Given the above reasoning, a checkbox is a better choice in those situations. The toggle is still a good choice in situations where immediate visual feedback is given, like when a dropcap is applied, or when you toggle image croppping for gallery items. In those situations, the pairing of the boolean nature and the larger touch target makes the switch feel more substantial. With this PR I'm trying to take into account all the past discussions on the topic, and I know there have been many. Lay your thoughts on me. * docs: Tidy up FormToggle docs * Removed unused variables and withInstanceId. Thanks for the feedback, @paulwilde. * Try a fix for the legitimate failing e2e test. Not sure if this is kosher, as the ID will change if additional checkboxes are added. * Fix e2e test really.
WordPress · Aug 8, 2018 · ee6060f · ee6060f
1 parent 09dc71c
commit ee6060f
Show file tree

Hide file tree

Showing 10 changed files with 80 additions and 58 deletions.
diff --git a/edit-post/assets/stylesheets/main.scss b/edit-post/assets/stylesheets/main.scss
@@ -121,13 +121,14 @@ body.gutenberg-editor-page {
 	}
 }
 
-// Override core input styles to provide ones consistent with Gutenberg
-// @todo submit as upstream patch as well
+// Override core input styles to provide styles consistent with Gutenberg.
+// Upstream ticket for redesigned input and styles in general: https://core.trac.wordpress.org/ticket/44749
+// Upstream ticket for redesigned checkbox: https://core.trac.wordpress.org/ticket/35596
 .edit-post-sidebar,
 .editor-post-publish-panel,
 .editor-block-list__block,
 .components-popover {
-	.input-control, // upstream name is .regular-text
+	.input-control, // Upstream name is `.regular-text`.
 	input[type="text"],
 	input[type="search"],
 	input[type="radio"],
@@ -168,6 +169,32 @@ body.gutenberg-editor-page {
 			outline-offset: 0;
 		}
 	}
+
+	input[type="checkbox"] {
+		border: $border-width + 1 solid $dark-gray-300;
+		border-radius: $radius-round-rectangle / 2;
+		margin-right: 12px;
+		transition: none;
+
+		&:focus {
+			border-color: $dark-gray-300;
+			box-shadow: 0 0 0 1px $dark-gray-300;
+		}
+
+		&:checked {
+			background: theme(toggle);
+			border-color: theme(toggle);
+		}
+
+		&:checked:focus {
+			box-shadow: 0 0 0 2px $dark-gray-500;
+		}
+
+		&::before {
+			margin: -4px 0 0 -5px;
+			color: $white;
+		}
+	}
 }
 
 // Placeholder colors

diff --git a/packages/components/src/base-control/style.scss b/packages/components/src/base-control/style.scss
@@ -1,7 +1,3 @@
-.components-base-control {
-	margin: 0 0 1.5em;
-}
-
 .components-base-control__label {
 	display: block;
 	margin-bottom: 5px;

diff --git a/packages/components/src/checkbox-control/style.scss b/packages/components/src/checkbox-control/style.scss
@@ -1,4 +1,3 @@
 .components-checkbox-control__input[type="checkbox"] {
 	margin-top: 0;
-	margin-right: 6px;
 }
diff --git a/packages/components/src/form-toggle/README.md b/packages/components/src/form-toggle/README.md
@@ -1,5 +1,21 @@
 # FormToggle
 
+The Form Toggle Control is a switch that should be **used when the effect is boolean and instant**. The Form Toggle Control is a complement to the Checkbox Control.
+
+Use Form Toggle when:
+
+- The effect of the switch is immediately visible to the user. For example: when applying a drop-cap to text, the actual drop-cap is immediately activated.
+- There are only two states for the switch: on or off.
+
+Do **not** use:
+
+- When the control is part of a group of other related controls (multiple choice).
+- When the effect of flipping the switch is not instantaneous.
+
+When Form Toggle component is not appropriate, use the Checkbox Control.
+
+Note: it is recommended that you pair the switch control with contextual help text, for example `checked ? __( 'Thumbnails are cropped to align.' ) : __( 'Thumbnails are not cropped.' )`.
+
 ## Usage
 
 ```jsx

diff --git a/packages/editor/src/components/post-comments/index.js b/packages/editor/src/components/post-comments/index.js
@@ -2,24 +2,20 @@
  * WordPress dependencies
  */
 import { __ } from '@wordpress/i18n';
-import { FormToggle } from '@wordpress/components';
-import { withInstanceId, compose } from '@wordpress/compose';
+import { CheckboxControl } from '@wordpress/components';
+import { compose } from '@wordpress/compose';
 import { withSelect, withDispatch } from '@wordpress/data';
 
-function PostComments( { commentStatus = 'open', instanceId, ...props } ) {
+function PostComments( { commentStatus = 'open', ...props } ) {
 	const onToggleComments = () => props.editPost( { comment_status: commentStatus === 'open' ? 'closed' : 'open' } );
 
-	const commentsToggleId = 'allow-comments-toggle-' + instanceId;
-
-	return [
-		<label key="label" htmlFor={ commentsToggleId }>{ __( 'Allow Comments' ) }</label>,
-		<FormToggle
-			key="toggle"
+	return (
+		<CheckboxControl
+			label={ __( 'Allow Comments' ) }
 			checked={ commentStatus === 'open' }
 			onChange={ onToggleComments }
-			id={ commentsToggleId }
-		/>,
-	];
+		/>
+	);
 }
 
 export default compose( [
@@ -31,5 +27,4 @@ export default compose( [
 	withDispatch( ( dispatch ) => ( {
 		editPost: dispatch( 'core/editor' ).editPost,
 	} ) ),
-	withInstanceId,
 ] )( PostComments );
diff --git a/packages/editor/src/components/post-pending-status/index.js b/packages/editor/src/components/post-pending-status/index.js
@@ -2,27 +2,25 @@
  * WordPress dependencies
  */
 import { __ } from '@wordpress/i18n';
-import { FormToggle } from '@wordpress/components';
+import { CheckboxControl } from '@wordpress/components';
 import { withSelect, withDispatch } from '@wordpress/data';
-import { withInstanceId, compose } from '@wordpress/compose';
+import { compose } from '@wordpress/compose';
 
 /**
  * Internal dependencies
  */
 import PostPendingStatusCheck from './check';
 
-export function PostPendingStatus( { instanceId, status, onUpdateStatus } ) {
-	const pendingId = 'pending-toggle-' + instanceId;
+export function PostPendingStatus( { status, onUpdateStatus } ) {
 	const togglePendingStatus = () => {
 		const updatedStatus = status === 'pending' ? 'draft' : 'pending';
 		onUpdateStatus( updatedStatus );
 	};
 
 	return (
 		<PostPendingStatusCheck>
-			<label htmlFor={ pendingId }>{ __( 'Pending Review' ) }</label>
-			<FormToggle
-				id={ pendingId }
+			<CheckboxControl
+				label={ __( 'Pending Review' ) }
 				checked={ status === 'pending' }
 				onChange={ togglePendingStatus }
 			/>
@@ -39,5 +37,4 @@ export default compose(
 			dispatch( 'core/editor' ).editPost( { status } );
 		},
 	} ) ),
-	withInstanceId
 )( PostPendingStatus );
diff --git a/packages/editor/src/components/post-pingbacks/index.js b/packages/editor/src/components/post-pingbacks/index.js
@@ -2,24 +2,20 @@
  * WordPress dependencies
  */
 import { __ } from '@wordpress/i18n';
-import { FormToggle } from '@wordpress/components';
+import { CheckboxControl } from '@wordpress/components';
 import { withSelect, withDispatch } from '@wordpress/data';
-import { withInstanceId, compose } from '@wordpress/compose';
+import { compose } from '@wordpress/compose';
 
-function PostPingbacks( { pingStatus = 'open', instanceId, ...props } ) {
+function PostPingbacks( { pingStatus = 'open', ...props } ) {
 	const onTogglePingback = () => props.editPost( { ping_status: pingStatus === 'open' ? 'closed' : 'open' } );
 
-	const pingbacksToggleId = 'allow-pingbacks-toggle-' + instanceId;
-
-	return [
-		<label key="label" htmlFor={ pingbacksToggleId }>{ __( 'Allow Pingbacks & Trackbacks' ) }</label>,
-		<FormToggle
-			key="toggle"
+	return (
+		<CheckboxControl
+			label={ __( 'Allow Pingbacks & Trackbacks' ) }
 			checked={ pingStatus === 'open' }
 			onChange={ onTogglePingback }
-			id={ pingbacksToggleId }
-		/>,
-	];
+		/>
+	);
 }
 
 export default compose( [
@@ -31,5 +27,4 @@ export default compose( [
 	withDispatch( ( dispatch ) => ( {
 		editPost: dispatch( 'core/editor' ).editPost,
 	} ) ),
-	withInstanceId,
 ] )( PostPingbacks );
diff --git a/packages/editor/src/components/post-sticky/index.js b/packages/editor/src/components/post-sticky/index.js
@@ -2,26 +2,22 @@
  * WordPress dependencies
  */
 import { __ } from '@wordpress/i18n';
-import { FormToggle } from '@wordpress/components';
+import { CheckboxControl } from '@wordpress/components';
 import { withSelect, withDispatch } from '@wordpress/data';
-import { withInstanceId, compose } from '@wordpress/compose';
+import { compose } from '@wordpress/compose';
 
 /**
  * Internal dependencies
  */
 import PostStickyCheck from './check';
 
-export function PostSticky( { onUpdateSticky, postSticky = false, instanceId } ) {
-	const stickyToggleId = 'post-sticky-toggle-' + instanceId;
-
+export function PostSticky( { onUpdateSticky, postSticky = false } ) {
 	return (
 		<PostStickyCheck>
-			<label htmlFor={ stickyToggleId }>{ __( 'Stick to the Front Page' ) }</label>
-			<FormToggle
-				key="toggle"
+			<CheckboxControl
+				label={ __( 'Stick to the Front Page' ) }
 				checked={ postSticky }
 				onChange={ () => onUpdateSticky( ! postSticky ) }
-				id={ stickyToggleId }
 			/>
 		</PostStickyCheck>
 	);
@@ -40,5 +36,4 @@ export default compose( [
 			},
 		};
 	} ),
-	withInstanceId,
 ] )( PostSticky );
diff --git a/packages/editor/src/components/post-taxonomies/style.scss b/packages/editor/src/components/post-taxonomies/style.scss
@@ -1,27 +1,27 @@
 .editor-post-taxonomies__hierarchical-terms-choice {
-	margin-bottom: 5px;
+	margin-bottom: 8px;
 }
 
 .editor-post-taxonomies__hierarchical-terms-input[type="checkbox"] {
 	margin-top: 0;
 }
 
 .editor-post-taxonomies__hierarchical-terms-subchoices {
-	margin-top: 5px;
+	margin-top: 8px;
 	margin-left: $panel-padding;
 }
 
 .components-button.editor-post-taxonomies__hierarchical-terms-submit,
 .components-button.editor-post-taxonomies__hierarchical-terms-add {
-	margin-top: 10px;
+	margin-top: 12px;
 }
 
 .editor-post-taxonomies__hierarchical-terms-label {
 	display: inline-block;
-	margin-top: 10px;
+	margin-top: 12px;
 }
 
 .editor-post-taxonomies__hierarchical-terms-input {
-	margin-top: 5px;
+	margin-top: 8px;
 	width: 100%;
 }
diff --git a/test/e2e/specs/change-detection.test.js b/test/e2e/specs/change-detection.test.js
@@ -92,7 +92,9 @@ describe( 'Change detection', () => {
 
 		// Toggle post as sticky (not persisted for autosave).
 		await ensureSidebarOpened();
-		await page.click( '[id^="post-sticky-toggle-"]' );
+
+		const postStickyToggleButton = ( await page.$x( "//label[contains(text(), 'Stick to the Front Page')]" ) )[ 0 ];
+		await postStickyToggleButton.click( 'button' );
 
 		// Force autosave to occur immediately.
 		await Promise.all( [