fix(styles): fix overflow styling on Radio Group and Checkbox and align styles with Pattern Library #1110

yhafez · 2023-07-16T15:44:29Z

Closes #627
Relates to #949
Relates to #1034

Relates to Walnut #5739

Current behavior:
https://github.com/dequelabs/cauldron/assets/60084578/b94f6133-0c7c-4368-9277-6736618301f9

With .Checkbox, .Radio flexwrap changed to nowrap:
https://github.com/dequelabs/cauldron/assets/60084578/ee399c8d-88c7-4b37-be61-2e2b4403bb21

w/ 4th option under wrapping line:
https://github.com/dequelabs/cauldron/assets/60084578/4dbaa581-63a3-41e5-9a02-d0033e09c6e0

With .Checkbox, .Radio flexwrap changed to nowrap and .Radio__overlay given align-self of flex-start (implemented):
https://github.com/dequelabs/cauldron/assets/60084578/6142d258-e011-451f-9427-bff88b6bcee9

w/ 4th option under wrapping line:
https://github.com/dequelabs/cauldron/assets/60084578/fbe68121-6973-4831-af4b-12316976020d

github-actions · 2023-07-16T15:45:28Z

Preview branch generated at https://radio-group-wrap.d1gko6en628vir.amplifyapp.com

scurker

Checkbox/radio share styles, we cannot make a change to one without changing both, see below screenshot:

yhafez · 2023-07-17T14:58:46Z

Checkbox/radio share styles, we cannot make a change to one without changing both, see below screenshot:

Addressed!

scurker · 2023-07-17T20:25:19Z

After a quick zoom meeting with @yhafez and @dequelabs/cauldron-design, we settled on the following:

Make sure the final designs for RadioGroup and Checkbox match the Pattern Library
- This should partially address Sync checkboxes with UXpin #949 and fully address Sync RadioGroup radio items with UXPin #1034
Change the outer wrapper to use a class with something like Checkbox__wrap and RadioGroup__wrap

scurker

Good work! Just a few nits to fix.

packages/styles/forms.css

scurker · 2023-07-20T17:41:07Z

Update from @dequelabs/cauldron-design

There should be a global var accent-primary-disabled set to #78a6d8 as we will be using it elsewhere.

Co-authored-by: Jason <[email protected]>

bobbyomari · 2023-07-20T19:08:36Z

@yhafez here are my observations...

checkbox dark theme:

the active fill on touch down should be accent-light @ 25% opacity
the active fill on touch down in error state should be accent-danger @ 25% opacity
the .Field_labelDescription should be color accent-light
Set margin-top: var(--space-smallest); for .error on checkbox with description. I think it strikes a better visual balance with how the code is structured (see attached screenshots, the one with more spacing is 8px). Looks fine with what it's at on
If label is being used with checkboxes, can we set the field margin-bottom to var(--space-smallest)? This also comes to play with radios.

checkbox light theme:

the active fill on touch down should be accent-primary @ 25% opacity
the active fill on touch down in error state should be error @ 25% opacity
same as dark theme... set margin-top: var(--space-smallest); for .error on checkbox with description

radio dark theme:

it appears the radio focus is not circular, it's a rectangle
.Field_label padding-bottom should be var(--space-smallest)
related to above: should we use margin-bottom instead of padding-bottom?
like dark checkboxes, the active fill on touch down should be the same
on inline layout, remove margin-bottom from .Radio_wrap

radio light theme:

same radio focus issue as dark theme
same .Field_label request as dark theme
like light checkboxes, the active fill on touch down should be the same
same inline layout margin-bottom change as dark theme radios

current implementation with `margin-top: 4px;`

updated implementation with `margin-top: 8px;`

bobbyomari · 2023-07-20T23:40:50Z

I already sent these changes to @yhafez but wanted to document them here as well for a 2nd PR review...

Checkboxes

I noticed on the error state, when the checkbox is empty, the active fill is correct, but when the error checkbox is checked, the active fill stays red. Is there way to make this change to red active fill only when the checkbox is unchecked and in an error state? This applies to light and dark theme.

Radios

The label for "do you like pizzas?" is now no longer font-weight: 500;
Some discrepancies between Cauldron in UXPin vs development...
-- The space between the radio label and description should be var(--space-half);
-- The space between radio groups should be var(--space-small);
-- This helps create some better visual balance between what description belongs to what radio field. Below are screenshots to share what that change looks like.

Radio with long description

Radio with short description

scurker

Looking good! Just a last few code changes needed.

docs/pages/components/Checkbox.mdx

packages/styles/forms.css

scurker

Code LGTM, 2 questions. Let's get @dequelabs/cauldron-design to give a thumbs up too.

packages/styles/forms.css

Co-authored-by: Jason <[email protected]>

scurker

Good work @yhafez!

github-actions · 2023-07-21T18:16:58Z

Preview branch generated at https://radio-group-wrap.d1gko6en628vir.amplifyapp.com

tbusillo · 2023-07-27T17:25:39Z

packages/styles/forms.css

+  border-top: 1px solid var(--field-error-border-color);
  text-align: left;
-  font-size: var(--text-size-smallest);
  font-weight: var(--font-weight-normal);
-  padding-left: var(--space-half);
+  font-size: var(--text-size-smallest);
+  margin-top: var(--space-half);
+  margin-left: calc(var(--icon-size) + 2px + var(--space-half));
+  padding: var(--space-half) 0;


@yhafez This is the change that is doing this.

yhafez added 2 commits July 16, 2023 16:40

fix: fix overfllow bug

cd0b50d

fix: fix overflow bug

ba3462c

yhafez requested a review from a team as a code owner July 16, 2023 15:44

chore: fix label

5bd19b1

scurker requested changes Jul 17, 2023

View reviewed changes

chore: address pr review

d47de1d

yhafez changed the title ~~fix: fix overflow styling on Radio Group~~ fix: fix overflow styling on Radio Group and Checkbox Jul 17, 2023

scurker changed the title ~~fix: fix overflow styling on Radio Group and Checkbox~~ fix(styles): fix overflow styling on Radio Group and Checkbox Jul 17, 2023

This was linked to issues Jul 17, 2023

Sync checkboxes with UXpin #949

Closed

Sync RadioGroup radio items with UXPin #1034

Closed

feat: sync up radio and checkbox styles with uxpin pattern library

d666d1a

yhafez changed the title ~~fix(styles): fix overflow styling on Radio Group and Checkbox~~ fix(styles): fix overflow styling on Radio Group and Checkbox and align styles with Pattern Library Jul 19, 2023

yhafez and others added 2 commits July 20, 2023 00:14

chore: change wrapper classed to Checbox__wrap and Radio__wrap

8a1d83a

Merge branch 'develop' into radio-group-wrap

f00458e

yhafez requested a review from scurker July 20, 2023 14:38

scurker requested changes Jul 20, 2023

View reviewed changes

yhafez and others added 2 commits July 20, 2023 22:07

Update packages/styles/forms.css

6ae03f9

Co-authored-by: Jason <[email protected]>

Update packages/styles/forms.css

fbd3eba

Co-authored-by: Jason <[email protected]>

yhafez added 4 commits July 20, 2023 22:23

chore: address pr review

9808d2c

feat: revise form.css to use more component-level variables

de16132

chore: address pr review

278a27d

chore: address pr review

5993768

chore: address pr review

41fd61b

yhafez requested a review from a team as a code owner July 21, 2023 11:57

yhafez added 3 commits July 21, 2023 15:14

chore: address pr review

25a6269

chore: address pr review

e8a335f

chore: fix a11y job

cdeceeb

scurker requested changes Jul 21, 2023

View reviewed changes

docs/pages/components/Checkbox.mdx Outdated Show resolved Hide resolved

packages/styles/forms.css Outdated Show resolved Hide resolved

packages/styles/forms.css Outdated Show resolved Hide resolved

packages/styles/forms.css Outdated Show resolved Hide resolved

yhafez added 6 commits July 21, 2023 18:22

chore: address pr review

e82c038

chore: address pr review

c648f1a

chore: address pr review

06ec325

chore: address pr review

33f2899

chore: address pr review

d89bf65

chore: address pr review

6a48424

scurker reviewed Jul 21, 2023

View reviewed changes

packages/styles/forms.css Outdated Show resolved Hide resolved

packages/styles/forms.css Show resolved Hide resolved

yhafez and others added 3 commits July 21, 2023 19:51

chore: address pr review

1829f02

Update packages/styles/forms.css

a88ae17

Co-authored-by: Jason <[email protected]>

chore: revert change

574ddd0

scurker approved these changes Jul 21, 2023

View reviewed changes

yhafez merged commit 4f8a9ba into develop Jul 21, 2023

yhafez deleted the radio-group-wrap branch July 21, 2023 18:16

github-actions bot mentioned this pull request Jul 25, 2023

chore(cauldron): Release 5.5.1 #1123

Merged

scurker mentioned this pull request Jul 25, 2023

fix: updated radio and checkboxes so labels wrap below #919

Closed

tbusillo reviewed Jul 27, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(styles): fix overflow styling on Radio Group and Checkbox and align styles with Pattern Library #1110

fix(styles): fix overflow styling on Radio Group and Checkbox and align styles with Pattern Library #1110

yhafez commented Jul 16, 2023 •

edited

Loading

github-actions bot commented Jul 16, 2023

scurker left a comment

yhafez commented Jul 17, 2023

scurker commented Jul 17, 2023

scurker left a comment

scurker commented Jul 20, 2023

bobbyomari commented Jul 20, 2023

bobbyomari commented Jul 20, 2023

scurker left a comment

scurker left a comment

scurker left a comment

github-actions bot commented Jul 21, 2023

tbusillo Jul 27, 2023

fix(styles): fix overflow styling on Radio Group and Checkbox and align styles with Pattern Library #1110

fix(styles): fix overflow styling on Radio Group and Checkbox and align styles with Pattern Library #1110

Conversation

yhafez commented Jul 16, 2023 • edited Loading

github-actions bot commented Jul 16, 2023

scurker left a comment

Choose a reason for hiding this comment

yhafez commented Jul 17, 2023

scurker commented Jul 17, 2023

scurker left a comment

Choose a reason for hiding this comment

scurker commented Jul 20, 2023

bobbyomari commented Jul 20, 2023

checkbox dark theme:

checkbox light theme:

radio dark theme:

radio light theme:

current implementation with margin-top: 4px;

updated implementation with margin-top: 8px;

bobbyomari commented Jul 20, 2023

Checkboxes

Radios

Radio with long description

Radio with short description

scurker left a comment

Choose a reason for hiding this comment

scurker left a comment

Choose a reason for hiding this comment

scurker left a comment

Choose a reason for hiding this comment

github-actions bot commented Jul 21, 2023

tbusillo Jul 27, 2023

Choose a reason for hiding this comment

yhafez commented Jul 16, 2023 •

edited

Loading

current implementation with `margin-top: 4px;`

updated implementation with `margin-top: 8px;`