FormControl refactor/cleanup by langermank · Pull Request #2114 · primer/css

langermank · 2022-06-02T16:56:42Z

What are you trying to accomplish?

Slight refactor and simplification for all Form components (based off the new app header styles).

Instead of using grid slots for leading/trailing items, I'm using position: absolute and specific inline padding for fields.
Checkbox and Radio have been refactored to use pure CSS instead of SVG for pseudo field styling

This will correspond with the new Rails forms framework, but I added some stories for easier visual testing.

Can these changes ship as is?

This markup hasn't been released yet in PVC/dotcom, so we're safe to make breaking changes.

Yes, this PR does not depend on additional changes. 🚢

TODO:

Test and cleanup animations/transitions
Add styles for Windows High Contrast + test

changeset-bot · 2022-06-02T16:56:44Z

🦋 Changeset detected

Latest commit: d3c516c

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@primer/css	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

langermank · 2022-06-19T18:35:22Z

- &.FormControl-fieldWrap--input-trailingAction {
- grid-template-columns: var(--base-size-16, 16px) minmax(0, auto) min-content;
+ // show vertical divider line between field and button
+ &::before {


Should this divider line be an optional prop?

Yes, if you can describe when the line should/should not be used.
Also: we don't have this style as an option in PRC. Is this to support a legacy style? Or is this a current pattern we need to support?

This came from @vdepizzol design for the new AppHeader, so a new pattern. We were thinking it might make sense to recommend it for fields that have a secondary action, not a clear button as you see in my screenshot. We can help address this in the form interface guidelines once this ships!

So if this isn't a clear button, what does it do?

…heckbox-radio

langermank · 2022-06-29T23:20:25Z

- }
+ // extend touch target
+ &::after {
+ @include minTouchTarget(var(--primer-control-medium-size, 32px), var(--primer-control-medium-size, 32px));


@mperrotti I extended the touch target to handle that gap between label/input (and to extend a bit beyond the tiny input) 🌸

…heckbox-radio

langermank · 2022-06-30T21:17:47Z

Windows High Contrast mode testing from @ichelsea:

jonrohan · 2022-06-30T21:35:58Z

 input[type='radio'],
 input[type='checkbox'] {
- transition: 80ms cubic-bezier(0.33, 1, 0.68, 1);
- transition-property: color, background-color, box-shadow, border-color;


This might fix a bunch of the transition bugs we've seen? 🤩

I think so.. I feel a little worried deleting this but its probably fine? Just a transition 😅

I don't believe there was functionality relying on this

Katie Langerman added 3 commits June 1, 2022 19:30

checkbox animation adjustment
92c4b3f

radio styles
0cfcc31

disabled state
9518046

Katie Langerman added 7 commits June 18, 2022 13:48

Merge branch 'main' ofhttps://github.com/primer/cssinto checkbox-radio
c7135a1

pull styles from app header
8359f2e

cleanup spacing/alignment
bfb107f

add checkbox focus styles
d269f1c

stories + radio focus styles
343c347

convert to css check
31b8d0d

refactor to pure *CSS* :)
8e1c6bb

langermank temporarily deployed to github-pages June 19, 2022 18:07 Inactive

Katie Langerman added 2 commits June 19, 2022 18:10

configure multi radio story
fee87e0

lint
9a356db

langermank changed the title ~~Checkbox + Radio updates~~FormControl refactor/cleanupJun 19, 2022

langermank temporarily deployed to github-pages June 19, 2022 18:32 Inactive

langermank commented Jun 19, 2022
View reviewed changes

Comment threadsrc/forms/FormControl.scss Outdated

langermank commented Jun 19, 2022
View reviewed changes

Create neat-sloths-march.md
0be7266

github-actionsBot added the patch release label Jun 19, 2022

langermank marked this pull request as ready for review June 19, 2022 18:37

langermank requested a review from a team as a code owner June 19, 2022 18:37

langermank requested review from camertron, simurai and vdepizzol June 19, 2022 18:37

langermank temporarily deployed to github-pages June 19, 2022 18:43 Inactive

mperrotti reviewed Jun 20, 2022
View reviewed changes

Katie Langerman added 2 commits June 21, 2022 14:58

snappier animations
876fa28

Merge branch 'checkbox-radio' ofhttps://github.com/primer/cssinto c…
5bb40a3
…heckbox-radio

langermank temporarily deployed to github-pages June 21, 2022 15:04 Inactive

Katie Langerman added 2 commits June 29, 2022 23:16

adjust disabled checked styles
c39c876

Merge branch 'checkbox-radio' ofhttps://github.com/primer/cssinto c…
ff5a45c
…heckbox-radio

langermank commented Jun 29, 2022
View reviewed changes

langermank temporarily deployed to github-pages June 29, 2022 23:22 Inactive

github-actionsBot temporarily deployed to Storybook Preview June 29, 2022 23:23 Inactive

Katie Langermanand others added 3 commits June 29, 2022 23:47

bump primitives
a149ad6

Stylelint auto-fixes
0b3e692

Update neat-sloths-march.md
c63fdbd

github-actionsBot temporarily deployed to Storybook Preview June 29, 2022 23:55 Inactive

langermank temporarily deployed to github-pages June 30, 2022 00:01 Inactive

github-actionsBot temporarily deployed to Storybook Preview June 30, 2022 00:01 Inactive

Katie Langerman added 2 commits June 30, 2022 17:16

temp add prefix for testing
cbf6b34

Merge branch 'checkbox-radio' ofhttps://github.com/primer/cssinto c…
74d89a7
…heckbox-radio

langermank temporarily deployed to github-pages June 30, 2022 17:22 Inactive

github-actionsBot temporarily deployed to Storybook Preview June 30, 2022 17:23 Inactive

test windows high contrast
02ae03d

langermank temporarily deployed to github-pages June 30, 2022 19:17 Inactive

github-actionsBot temporarily deployed to Storybook Preview June 30, 2022 19:18 Inactive

Katie Langerman added 2 commits June 30, 2022 21:20

cleanup prefix for final build
5067381

Merge branch 'main' into checkbox-radio
d3c516c

github-actionsBot temporarily deployed to Storybook Preview June 30, 2022 21:27 Inactive

langermank temporarily deployed to github-pages June 30, 2022 21:29 Inactive

github-actionsBot temporarily deployed to Storybook Preview June 30, 2022 21:29 Inactive

jonrohan approved these changes Jun 30, 2022
View reviewed changes

langermank merged commit facb968 into mainJun 30, 2022

langermank deleted the checkbox-radio branch June 30, 2022 21:37

primer-css mentioned this pull request Jun 30, 2022
Release Tracking #2133
Merged

This was referenced Jul 1, 2022
New form classes primer/rails_forms#9
Merged
Ship new form styles primer/rails_forms#11
Closed

Uh oh!

Conversation

langermank commented Jun 2, 2022• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What are you trying to accomplish?

Can these changes ship as is?

Uh oh!

changeset-botBot commented Jun 2, 2022• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🦋 Changeset detected

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

langermank commented Jun 30, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

langermank commented Jun 2, 2022•
edited
Loading

changeset-botBot commented Jun 2, 2022•
edited
Loading