Fix: Update styles for checkbox and radio controls. #61696

vipul0425 · 2024-05-15T21:00:56Z

What?

Fixes issue: #61652

Why?

The current spacing between the checkbox input and its label is inconsistent with that of the radio buttons, even though they have the same size.

How?

This PR updates the stylings for the checkbox control and radio control to make them consistent.

Testing Instructions

Open any page or post in the editor and open its settings panel.
Click on the published or discussions buttons and another tab should appear where one can test the checkbox and radio controls simultaneously.

Testing Instructions for Keyboard

nil

Screenshots or screencast

github-actions · 2024-05-15T21:01:12Z

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: vipul0425 <vipulgupta003@git.wordpress.org>
Co-authored-by: mirka <0mirka00@git.wordpress.org>
Co-authored-by: jameskoster <jameskoster@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

jameskoster · 2024-05-16T13:22:13Z

Thanks for the PR, it will be nice to align the spacing here :)

I'd welcome feedback from others, but the balance doesn't feel quite right to me:

Specifically, it's hierarchically awkward for the space between input + label to be larger than the space between each radio option. Perhaps 8px spacing between input + label, and 12px spacing between options would work better?

If this is preferable we should obviously update the checkbox component to match.

I appreciate the latter part is slightly out of scope, but it's indirectly related so probably fine to tweak here.

What do you think?

vipul0425 · 2024-05-17T17:29:39Z

Hi @jameskoster,
Thanks for reviewing it. I have updated the spacing between the input and label to 8px.

jameskoster · 2024-05-20T10:02:45Z

@vipul0425 thanks! How did you feel about increasing the spacing between radios to 12px?

…-61652

vipul0425 · 2024-06-03T15:55:29Z

Hi @jameskoster, Sorry for replying that late. I have revised the spacing between radio controls to 12px and also rebased the branch.
Thanks.

mirka

I had some tweaks and questions, but on the whole this is looking good 👍

mirka · 2024-06-03T20:32:42Z

packages/editor/src/components/post-comments/index.js

+					{ __(
+						'Visitors cannot add new comments or replies. Existing comments remain visible.'
+					) }


Was this committed by accident?

No, Actually when we updated the spacing between the spacing between the input and label to 12px then it was not looking visually appealing so converted it to a single line. But I think now its not required as we have revised it to 8px. Will update this in next commit.

Wait, is the extra wrapper remaining intentional?

Oh sorry @mirka it was a mistake 😓 Will update this.

mirka · 2024-06-04T00:45:33Z

packages/components/src/checkbox-control/style.scss

@@ -55,7 +57,7 @@
 .components-checkbox-control__input-container {
 	position: relative;
 	display: inline-block;
-	margin-right: $grid-unit-15;
+	margin-right: var(--checkbox-input-margin);


This change wasn't reflected in the help text margin, so I extracted it into a CSS var (f050242).

mirka · 2024-06-04T00:47:15Z

packages/components/src/checkbox-control/style.scss

+}
+
+p.components-base-control__help:has(.components-checkbox-control__help) {
+	margin-top: $grid-unit-05;


Was this change discussed somewhere? The margin-top is still the standard 8px on the RadioControl, so not sure where this came from. I would prefer not to override this BaseControl style if possible, since it's a standard value across many components.

To make it look consistent with the radio controls I have updated this to 4px. Let me know if we need to change this.

I see. That small text under those radio controls are custom spans injected by the consumer, and not the help text of the RadioControl component. The help text of the RadioControl is just a single chunk of text underneath all the options of a radio group.

The point is, we shouldn't be basing this decision (help text margin of the CheckboxControl) on a non-standard RadioControl use case like that "Status & visibility" panel.

For the scope of this PR, I would recommend removing this override of the CheckboxControl help text margin.

(No strong opinion about what kind of consistency improvements you might want to make at the consumer level in the "Status & visibility" panel itself.)

@mirka I agree thanks 🙇 . I have removed that style from the base component and added it specifically for the "status and visibility" panel I think it makes more sense there as radio and checkbox are similar so the spacing between the help text in both of these should look consistent in that panel. Is it good to go or we need to make any other changes here?

mirka

Looks good, thank you for all the follow-ups! Please add a changelog for the style changes in CheckboxControl and RadioControl, and we're good to merge 👍

…-61652

vipul0425 · 2024-06-07T10:46:02Z

@mirka Updated the changelog and rebased the branch 🙇 .

ellatrix · 2024-06-10T12:48:50Z

Is the a bug for or enhancement. Title and label don't match. Should this fix be included in 6.6 Beta?

vipul0425 · 2024-06-10T18:35:08Z

Yes @ellatrix It was for enhancement in the checkbox and radio controls. It can be included in 6.6 Beta.

* Fix: Update styles for checkbox and radio controls. * chore: Update margin-right for checkbox and radio controls in style.scss * fix: the sapcing b/w radio controls to 8px. * refactor: Revised the spacing b/w radio-controls to '12px'. * Fix checkbox help margin * Revert the line break change in post-comments. * fix: The changes as per feedback. * Doc: Updates the changelog.md. * Fixup changelog --------- Co-authored-by: vipul0425 <vipulgupta003@git.wordpress.org> Co-authored-by: mirka <0mirka00@git.wordpress.org> Co-authored-by: jameskoster <jameskoster@git.wordpress.org>

Fix: Update styles for checkbox and radio controls.

322019c

vipul0425 requested a review from ajitbohra as a code owner May 15, 2024 21:00

vipul0425 mentioned this pull request May 15, 2024

Make spacing consistent between radios / checkboxes #61652

Closed

jameskoster requested review from a team May 16, 2024 13:22

jameskoster added [Type] Enhancement A suggestion for improvement. [Package] Components /packages/components labels May 16, 2024

chore: Update margin-right for checkbox and radio controls in style.scss

d53df9f

mirka mentioned this pull request May 30, 2024

Editor: Align the Post Format control design with the rest of the post sidebar controls #62066

Merged

vipul0425 added 3 commits June 3, 2024 21:17

fix: the sapcing b/w radio controls to 8px.

c5e2b26

Merge branch 'trunk' of github.com:vipul0425/gutenberg into fix/issue…

9983e3b

…-61652

refactor: Revised the spacing b/w radio-controls to '12px'.

7f31e74

Fix checkbox help margin

f050242

mirka reviewed Jun 4, 2024

View reviewed changes

vipul0425 added 2 commits June 5, 2024 00:29

Revert the line break change in post-comments.

8c9d4d9

fix: The changes as per feedback.

c54e617

mirka approved these changes Jun 7, 2024

View reviewed changes

vipul0425 added 2 commits June 7, 2024 16:06

Merge branch 'trunk' of github.com:vipul0425/gutenberg into fix/issue…

e1b3fed

…-61652

Doc: Updates the changelog.md.

e4e378e

Fixup changelog

acd399b

mirka enabled auto-merge (squash) June 7, 2024 15:07

mirka merged commit bbdf1a7 into WordPress:trunk Jun 7, 2024
61 checks passed

github-actions bot added this to the Gutenberg 18.6 milestone Jun 7, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix: Update styles for checkbox and radio controls. #61696

Fix: Update styles for checkbox and radio controls. #61696

vipul0425 commented May 15, 2024

github-actions bot commented May 15, 2024 •

edited

Loading

jameskoster commented May 16, 2024

vipul0425 commented May 17, 2024

jameskoster commented May 20, 2024

vipul0425 commented Jun 3, 2024

mirka left a comment

mirka Jun 3, 2024

vipul0425 Jun 4, 2024

mirka Jun 4, 2024

vipul0425 Jun 6, 2024

mirka Jun 4, 2024

mirka Jun 4, 2024

vipul0425 Jun 4, 2024 •

edited

Loading

mirka Jun 4, 2024

vipul0425 Jun 6, 2024

mirka left a comment

vipul0425 commented Jun 7, 2024

ellatrix commented Jun 10, 2024

vipul0425 commented Jun 10, 2024

Fix: Update styles for checkbox and radio controls. #61696

Fix: Update styles for checkbox and radio controls. #61696

Conversation

vipul0425 commented May 15, 2024

What?

Why?

How?

Testing Instructions

Testing Instructions for Keyboard

Screenshots or screencast

github-actions bot commented May 15, 2024 • edited Loading

jameskoster commented May 16, 2024

vipul0425 commented May 17, 2024

jameskoster commented May 20, 2024

vipul0425 commented Jun 3, 2024

mirka left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vipul0425 Jun 4, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mirka left a comment

Choose a reason for hiding this comment

vipul0425 commented Jun 7, 2024

ellatrix commented Jun 10, 2024

vipul0425 commented Jun 10, 2024

github-actions bot commented May 15, 2024 •

edited

Loading

vipul0425 Jun 4, 2024 •

edited

Loading