From 16c726d909f47c01fc527091af1942a19da3ce43 Mon Sep 17 00:00:00 2001 From: Andrea Fercia Date: Tue, 2 Jul 2024 15:32:48 +0200 Subject: [PATCH 01/10] Add Stylelint rule to prevent usage of flex-direction *-reverse values. --- .stylelintrc.json | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/.stylelintrc.json b/.stylelintrc.json index df01978222e632..6628e831f8642a 100644 --- a/.stylelintrc.json +++ b/.stylelintrc.json @@ -4,6 +4,20 @@ "at-rule-empty-line-before": null, "at-rule-no-unknown": null, "comment-empty-line-before": null, + "declaration-property-value-allowed-list": [ + { + "flex-direction": [ + "row", + "column", + "inherit", + "initial", + "unset" + ] + }, + { + "message": "Avoid the flex-direction reverse values. For accessibility reasons, visual, reading, and DOM order must match. Only use the reverse values when they do not affect reading order, meaning, and interaction," + } + ], "declaration-property-value-disallowed-list": [ { "/.*/": [ "/--wp-components-color-/" ] @@ -18,7 +32,7 @@ "property-disallowed-list": [ [ "order" ], { - "message": "Avoid the order property. For accessibility reasons, visual, reading, and DOM order must match. Only use the order property when it does not affect reading order, meaning, and interaction" + "message": "Avoid the order property. For accessibility reasons, visual, reading, and DOM order must match. Only use the order property when it does not affect reading order, meaning, and interaction," } ], "rule-empty-line-before": null, From c1ce56960c4111b32295a8f30b17169c010d547e Mon Sep 17 00:00:00 2001 From: Andrea Fercia Date: Tue, 2 Jul 2024 16:51:53 +0200 Subject: [PATCH 02/10] Add Stylelint disable comments for valid cases or cases that should be refactored later. --- .../src/hooks/use-editor-wrapper-styles.native.scss | 1 + packages/block-library/src/media-text/style.native.scss | 2 ++ packages/block-library/src/navigation/style.scss | 2 ++ packages/block-library/src/page-list/style.scss | 1 + 4 files changed, 6 insertions(+) diff --git a/packages/block-editor/src/hooks/use-editor-wrapper-styles.native.scss b/packages/block-editor/src/hooks/use-editor-wrapper-styles.native.scss index c980253ee64251..2ae4ed8b86144c 100644 --- a/packages/block-editor/src/hooks/use-editor-wrapper-styles.native.scss +++ b/packages/block-editor/src/hooks/use-editor-wrapper-styles.native.scss @@ -5,6 +5,7 @@ } .use-editor-wrapper-styles--reversed { + /* stylelint-disable-next-line declaration-property-value-allowed-list -- This should be refactored to not use the column-reverse value. */ flex-direction: column-reverse; width: 100%; max-width: 580; diff --git a/packages/block-library/src/media-text/style.native.scss b/packages/block-library/src/media-text/style.native.scss index 07b584059203ed..4c0f80a5008a8d 100644 --- a/packages/block-library/src/media-text/style.native.scss +++ b/packages/block-library/src/media-text/style.native.scss @@ -7,6 +7,7 @@ $media-to-text: 12px; } .has-media-on-the-right { + /* stylelint-disable-next-line declaration-property-value-allowed-list -- This should be refactored to not use the row-reverse value. */ flex-direction: row-reverse; } @@ -14,6 +15,7 @@ $media-to-text: 12px; flex-direction: column; &.has-media-on-the-right { + /* stylelint-disable-next-line declaration-property-value-allowed-list -- This should be refactored to not use the column-reverse value. */ flex-direction: column-reverse; } } diff --git a/packages/block-library/src/navigation/style.scss b/packages/block-library/src/navigation/style.scss index 41606d5431e3fe..a6e3d519b572fe 100644 --- a/packages/block-library/src/navigation/style.scss +++ b/packages/block-library/src/navigation/style.scss @@ -432,6 +432,7 @@ button.wp-block-navigation-item__content { .wp-block-navigation__container { display: flex; flex-wrap: var(--navigation-layout-wrap, wrap); + /* stylelint-disable-next-line declaration-property-value-allowed-list -- This variable value does not use row-reverse or column-reverse. */ flex-direction: var(--navigation-layout-direction, initial); justify-content: var(--navigation-layout-justify, initial); align-items: var(--navigation-layout-align, initial); @@ -484,6 +485,7 @@ button.wp-block-navigation-item__content { .wp-block-navigation__responsive-container-content { display: flex; flex-wrap: var(--navigation-layout-wrap, wrap); + /* stylelint-disable-next-line declaration-property-value-allowed-list -- This variable value does not use row-reverse or column-reverse. */ flex-direction: var(--navigation-layout-direction, initial); justify-content: var(--navigation-layout-justify, initial); align-items: var(--navigation-layout-align, initial); diff --git a/packages/block-library/src/page-list/style.scss b/packages/block-library/src/page-list/style.scss index d06a6142573bec..f42548e367c859 100644 --- a/packages/block-library/src/page-list/style.scss +++ b/packages/block-library/src/page-list/style.scss @@ -2,6 +2,7 @@ .wp-block-navigation { .wp-block-page-list { display: flex; + /* stylelint-disable-next-line declaration-property-value-allowed-list -- This variable value does not use row-reverse or column-reverse. */ flex-direction: var(--navigation-layout-direction, initial); justify-content: var(--navigation-layout-justify, initial); align-items: var(--navigation-layout-align, initial); From db35911bb0445c5de8291304032f34609d72014e Mon Sep 17 00:00:00 2001 From: Andrea Fercia Date: Wed, 3 Jul 2024 10:38:42 +0200 Subject: [PATCH 03/10] Remove row-reverse from experimental checkbox form input. --- packages/block-library/src/form-input/edit.js | 44 ++++++++++++++----- packages/block-library/src/form-input/save.js | 16 +++++-- .../block-library/src/form-input/style.scss | 13 +++--- 3 files changed, 51 insertions(+), 22 deletions(-) diff --git a/packages/block-library/src/form-input/edit.js b/packages/block-library/src/form-input/edit.js index 97115d86fb3700..59d09b3bb80300 100644 --- a/packages/block-library/src/form-input/edit.js +++ b/packages/block-library/src/form-input/edit.js @@ -31,6 +31,9 @@ function InputFieldBlock( { attributes, setAttributes, className } ) { ref.current.focus(); } + // Note: radio inputs aren't implemented yet. + const isCheckboxOrRadio = type === 'checkbox' || type === 'radio'; + const controls = ( <> { 'hidden' !== type && ( @@ -107,17 +110,21 @@ function InputFieldBlock( { attributes, setAttributes, className } ) { 'is-label-inline': inlineLabel || 'checkbox' === type, } ) } > - - setAttributes( { label: newLabel } ) - } - aria-label={ label ? __( 'Label' ) : __( 'Empty label' ) } - data-empty={ label ? false : true } - placeholder={ __( 'Type the label for this input' ) } - /> + { ! isCheckboxOrRadio && ( + + setAttributes( { label: newLabel } ) + } + aria-label={ + label ? __( 'Label' ) : __( 'Empty label' ) + } + data-empty={ label ? false : true } + placeholder={ __( 'Type the label for this input' ) } + /> + ) } + { isCheckboxOrRadio && ( + + setAttributes( { label: newLabel } ) + } + aria-label={ + label ? __( 'Label' ) : __( 'Empty label' ) + } + data-empty={ label ? false : true } + placeholder={ __( 'Type the label for this input' ) } + /> + ) } ); diff --git a/packages/block-library/src/form-input/save.js b/packages/block-library/src/form-input/save.js index c408e06923ca9f..40eb37603ef6f4 100644 --- a/packages/block-library/src/form-input/save.js +++ b/packages/block-library/src/form-input/save.js @@ -55,6 +55,9 @@ export default function save( { attributes } ) { const blockProps = useBlockProps.save(); + // Note: radio inputs aren't implemented yet. + const isCheckboxOrRadio = type === 'checkbox' || type === 'radio'; + if ( 'hidden' === type ) { return ; } @@ -67,9 +70,11 @@ export default function save( { attributes } ) { 'is-label-inline': inlineLabel, } ) } > - - - + { ! isCheckboxOrRadio && ( + + + + ) } + { isCheckboxOrRadio && ( + + + + ) } { /* eslint-enable jsx-a11y/label-has-associated-control */ } diff --git a/packages/block-library/src/form-input/style.scss b/packages/block-library/src/form-input/style.scss index d45fc8d7f1f729..9ce6862d8ea024 100644 --- a/packages/block-library/src/form-input/style.scss +++ b/packages/block-library/src/form-input/style.scss @@ -15,16 +15,13 @@ } } - /* - Small tweak to left-align the checkbox. - Even though `:has` is not currently supported in Firefox, this is a small tweak - and does not affect the functionality of the block or the user's experience. - There will be a minor inconsistency between browsers. However, it's more important to provide - a better experience for 80+% of users, until Firefox catches up and supports `:has`. - */ &:has(input[type="checkbox"]) { width: fit-content; - flex-direction: row-reverse; + + .wp-block-form-input__input, + .wp-block-form-input__label-content { + margin: 0; + } } } From af868fafc38fb52cbcf20b516e214ebaec6eae62 Mon Sep 17 00:00:00 2001 From: Andrea Fercia Date: Wed, 3 Jul 2024 11:57:33 +0200 Subject: [PATCH 04/10] Add disable comment for the hooked blocks toggles. --- packages/block-editor/src/hooks/block-hooks.scss | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/block-editor/src/hooks/block-hooks.scss b/packages/block-editor/src/hooks/block-hooks.scss index 4d871233de482b..c8f2027483ccf3 100644 --- a/packages/block-editor/src/hooks/block-hooks.scss +++ b/packages/block-editor/src/hooks/block-hooks.scss @@ -4,6 +4,7 @@ * we need to right-align the toggle. */ .components-toggle-control .components-h-stack { + /* stylelint-disable-next-line declaration-property-value-allowed-list -- This should be refactored to not use the row-reverse value. */ flex-direction: row-reverse; } From 9efa055d4dd95834a3e7a548f909a259b226eede Mon Sep 17 00:00:00 2001 From: Andrea Fercia Date: Wed, 3 Jul 2024 14:29:08 +0200 Subject: [PATCH 05/10] Remove leftovers. --- packages/block-library/src/form-input/save.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/block-library/src/form-input/save.js b/packages/block-library/src/form-input/save.js index 40eb37603ef6f4..941c23dc2014d1 100644 --- a/packages/block-library/src/form-input/save.js +++ b/packages/block-library/src/form-input/save.js @@ -71,7 +71,7 @@ export default function save( { attributes } ) { } ) } > { ! isCheckboxOrRadio && ( - + ) } @@ -85,7 +85,7 @@ export default function save( { attributes } ) { style={ inputStyle } /> { isCheckboxOrRadio && ( - + ) } From 7b6abac07d42ca8e5e4a65ad11d1dd8346e1e7c2 Mon Sep 17 00:00:00 2001 From: Andrea Fercia Date: Mon, 22 Jul 2024 09:26:25 +0200 Subject: [PATCH 06/10] Fix typo. --- .stylelintrc.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.stylelintrc.json b/.stylelintrc.json index 6628e831f8642a..8a93f130d75be9 100644 --- a/.stylelintrc.json +++ b/.stylelintrc.json @@ -15,7 +15,7 @@ ] }, { - "message": "Avoid the flex-direction reverse values. For accessibility reasons, visual, reading, and DOM order must match. Only use the reverse values when they do not affect reading order, meaning, and interaction," + "message": "Avoid the flex-direction reverse values. For accessibility reasons, visual, reading, and DOM order must match. Only use the reverse values when they do not affect reading order, meaning, and interaction." } ], "declaration-property-value-disallowed-list": [ From 00f59d5bd3fed5b1ac15bf47aa469bb61f2fcd93 Mon Sep 17 00:00:00 2001 From: Andrea Fercia Date: Tue, 23 Jul 2024 15:56:42 +0200 Subject: [PATCH 07/10] Use regex. --- .stylelintrc.json | 8 +------- packages/block-library/src/navigation/style.scss | 2 -- packages/block-library/src/page-list/style.scss | 1 - 3 files changed, 1 insertion(+), 10 deletions(-) diff --git a/.stylelintrc.json b/.stylelintrc.json index 8a93f130d75be9..fb6bee7064c2bd 100644 --- a/.stylelintrc.json +++ b/.stylelintrc.json @@ -6,13 +6,7 @@ "comment-empty-line-before": null, "declaration-property-value-allowed-list": [ { - "flex-direction": [ - "row", - "column", - "inherit", - "initial", - "unset" - ] + "flex-direction": "/^(?!(row|column)-reverse).*$/" }, { "message": "Avoid the flex-direction reverse values. For accessibility reasons, visual, reading, and DOM order must match. Only use the reverse values when they do not affect reading order, meaning, and interaction." diff --git a/packages/block-library/src/navigation/style.scss b/packages/block-library/src/navigation/style.scss index a6e3d519b572fe..41606d5431e3fe 100644 --- a/packages/block-library/src/navigation/style.scss +++ b/packages/block-library/src/navigation/style.scss @@ -432,7 +432,6 @@ button.wp-block-navigation-item__content { .wp-block-navigation__container { display: flex; flex-wrap: var(--navigation-layout-wrap, wrap); - /* stylelint-disable-next-line declaration-property-value-allowed-list -- This variable value does not use row-reverse or column-reverse. */ flex-direction: var(--navigation-layout-direction, initial); justify-content: var(--navigation-layout-justify, initial); align-items: var(--navigation-layout-align, initial); @@ -485,7 +484,6 @@ button.wp-block-navigation-item__content { .wp-block-navigation__responsive-container-content { display: flex; flex-wrap: var(--navigation-layout-wrap, wrap); - /* stylelint-disable-next-line declaration-property-value-allowed-list -- This variable value does not use row-reverse or column-reverse. */ flex-direction: var(--navigation-layout-direction, initial); justify-content: var(--navigation-layout-justify, initial); align-items: var(--navigation-layout-align, initial); diff --git a/packages/block-library/src/page-list/style.scss b/packages/block-library/src/page-list/style.scss index f42548e367c859..d06a6142573bec 100644 --- a/packages/block-library/src/page-list/style.scss +++ b/packages/block-library/src/page-list/style.scss @@ -2,7 +2,6 @@ .wp-block-navigation { .wp-block-page-list { display: flex; - /* stylelint-disable-next-line declaration-property-value-allowed-list -- This variable value does not use row-reverse or column-reverse. */ flex-direction: var(--navigation-layout-direction, initial); justify-content: var(--navigation-layout-justify, initial); align-items: var(--navigation-layout-align, initial); From c4c55b21c0f8439b20e2084f5bd65e20401f5fc8 Mon Sep 17 00:00:00 2001 From: Aki Hamano Date: Fri, 26 Jul 2024 18:05:52 +0900 Subject: [PATCH 08/10] Revert changes to `form-input/*` --- packages/block-library/src/form-input/edit.js | 4 ++-- packages/block-library/src/form-input/save.js | 16 +++------------- packages/block-library/src/form-input/style.scss | 14 +++++++++----- 3 files changed, 14 insertions(+), 20 deletions(-) diff --git a/packages/block-library/src/form-input/edit.js b/packages/block-library/src/form-input/edit.js index 74296ddfaee810..a7869deb019aef 100644 --- a/packages/block-library/src/form-input/edit.js +++ b/packages/block-library/src/form-input/edit.js @@ -41,7 +41,6 @@ function InputFieldBlock( { attributes, setAttributes, className } ) { { 'checkbox' !== type && ( { @@ -49,10 +48,10 @@ function InputFieldBlock( { attributes, setAttributes, className } ) { inlineLabel: newVal, } ); } } + __nextHasNoMarginBottom /> ) } { @@ -60,6 +59,7 @@ function InputFieldBlock( { attributes, setAttributes, className } ) { required: newVal, } ); } } + __nextHasNoMarginBottom /> diff --git a/packages/block-library/src/form-input/save.js b/packages/block-library/src/form-input/save.js index 941c23dc2014d1..c408e06923ca9f 100644 --- a/packages/block-library/src/form-input/save.js +++ b/packages/block-library/src/form-input/save.js @@ -55,9 +55,6 @@ export default function save( { attributes } ) { const blockProps = useBlockProps.save(); - // Note: radio inputs aren't implemented yet. - const isCheckboxOrRadio = type === 'checkbox' || type === 'radio'; - if ( 'hidden' === type ) { return ; } @@ -70,11 +67,9 @@ export default function save( { attributes } ) { 'is-label-inline': inlineLabel, } ) } > - { ! isCheckboxOrRadio && ( - - - - ) } + + + - { isCheckboxOrRadio && ( - - - - ) } { /* eslint-enable jsx-a11y/label-has-associated-control */ } diff --git a/packages/block-library/src/form-input/style.scss b/packages/block-library/src/form-input/style.scss index 9ce6862d8ea024..f9e1753cf0a7b7 100644 --- a/packages/block-library/src/form-input/style.scss +++ b/packages/block-library/src/form-input/style.scss @@ -15,13 +15,17 @@ } } + /* + Small tweak to left-align the checkbox. + Even though `:has` is not currently supported in Firefox, this is a small tweak + and does not affect the functionality of the block or the user's experience. + There will be a minor inconsistency between browsers. However, it's more important to provide + a better experience for 80+% of users, until Firefox catches up and supports `:has`. + */ &:has(input[type="checkbox"]) { width: fit-content; - - .wp-block-form-input__input, - .wp-block-form-input__label-content { - margin: 0; - } + /* stylelint-disable-next-line declaration-property-value-allowed-list -- This should be refactored to not use the row-reverse value. */ + flex-direction: row-reverse; } } From d47840d9a6e77bf76116e51d5be5bc58603ab8c4 Mon Sep 17 00:00:00 2001 From: Aki Hamano Date: Fri, 26 Jul 2024 18:07:51 +0900 Subject: [PATCH 09/10] Revert changes to `form-input/edit.js` --- packages/block-library/src/form-input/edit.js | 48 +++++-------------- 1 file changed, 13 insertions(+), 35 deletions(-) diff --git a/packages/block-library/src/form-input/edit.js b/packages/block-library/src/form-input/edit.js index a7869deb019aef..855fdbf4684626 100644 --- a/packages/block-library/src/form-input/edit.js +++ b/packages/block-library/src/form-input/edit.js @@ -31,9 +31,6 @@ function InputFieldBlock( { attributes, setAttributes, className } ) { ref.current.focus(); } - // Note: radio inputs aren't implemented yet. - const isCheckboxOrRadio = type === 'checkbox' || type === 'radio'; - const controls = ( <> { 'hidden' !== type && ( @@ -41,6 +38,7 @@ function InputFieldBlock( { attributes, setAttributes, className } ) { { 'checkbox' !== type && ( { @@ -48,10 +46,10 @@ function InputFieldBlock( { attributes, setAttributes, className } ) { inlineLabel: newVal, } ); } } - __nextHasNoMarginBottom /> ) } { @@ -59,7 +57,6 @@ function InputFieldBlock( { attributes, setAttributes, className } ) { required: newVal, } ); } } - __nextHasNoMarginBottom /> @@ -112,21 +109,17 @@ function InputFieldBlock( { attributes, setAttributes, className } ) { 'is-label-inline': inlineLabel || 'checkbox' === type, } ) } > - { ! isCheckboxOrRadio && ( - - setAttributes( { label: newLabel } ) - } - aria-label={ - label ? __( 'Label' ) : __( 'Empty label' ) - } - data-empty={ label ? false : true } - placeholder={ __( 'Type the label for this input' ) } - /> - ) } + + setAttributes( { label: newLabel } ) + } + aria-label={ label ? __( 'Label' ) : __( 'Empty label' ) } + data-empty={ label ? false : true } + placeholder={ __( 'Type the label for this input' ) } + /> - { isCheckboxOrRadio && ( - - setAttributes( { label: newLabel } ) - } - aria-label={ - label ? __( 'Label' ) : __( 'Empty label' ) - } - data-empty={ label ? false : true } - placeholder={ __( 'Type the label for this input' ) } - /> - ) } ); From 5190d826e496be424bc53d1ab4cefb0fca726f04 Mon Sep 17 00:00:00 2001 From: Aki Hamano <54422211+t-hamano@users.noreply.github.com> Date: Fri, 26 Jul 2024 18:08:29 +0900 Subject: [PATCH 10/10] Update .stylelintrc.json Co-authored-by: Marin Atanasov <8436925+tyxla@users.noreply.github.com> --- .stylelintrc.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.stylelintrc.json b/.stylelintrc.json index fb6bee7064c2bd..663befa2e4ce06 100644 --- a/.stylelintrc.json +++ b/.stylelintrc.json @@ -26,7 +26,7 @@ "property-disallowed-list": [ [ "order" ], { - "message": "Avoid the order property. For accessibility reasons, visual, reading, and DOM order must match. Only use the order property when it does not affect reading order, meaning, and interaction," + "message": "Avoid the order property. For accessibility reasons, visual, reading, and DOM order must match. Only use the order property when it does not affect reading order, meaning, and interaction." } ], "rule-empty-line-before": null,