react icon indicating copy to clipboard operation
react copied to clipboard

fix(FormControl): allow required check boxes

Open francinelucca opened this issue 1 year ago • 5 comments

Relates to https://github.com/github/primer/issues/3470

FormControl by design does not allow individual Checkbox/Radio to be required. As per:

  • https://github.com/github/primer/issues/3470#issuecomment-2362018592
  • https://github.com/primer/react/pull/5027#discussion_r1787922994 there may be use cases where an individual Checkbox can be required individually or within a CheckboxGroup (since multiple checkboxes can be checked at once, opposed to radio buttons). This PR introduces functionality to FormControl allow that behavior to happen.

Changelog

New

  • Adds FormControl tests for required prop used with individual and group checkboxes

Changed

  • Updates choice group checks in FormControl to determine if the input is a Checkbox and pass down the required prop in which case
  • Updates CheckboxGroup Playground story to showcase one required Checkbox within a CheckboxGroup
  • Updates CheckboxGroup story snapshots to account for required change

Rollout strategy

  • [x] Patch release
  • [ ] Minor release
  • [ ] Major release; if selected, include a written rollout or migration plan
  • [ ] None; if selected, include a brief description as to why

Testing & Reviewing

  • Verify required checkbox within CheckboxGroup playground story, notice required prop on input in DOM.
  • There should be no visual changes to FormControl, Checkbox or CheckboxGroup
  • Verify Default FormControl story now properly marks required attribute on checkbox

Merge checklist

  • [x] Added/updated tests
  • [ ] Added/updated documentation
  • [x] Added/updated previews (Storybook)
  • [ ] Changes are SSR compatible
  • [x] Tested in Chrome
  • [ ] Tested in Firefox
  • [ ] Tested in Safari
  • [ ] Tested in Edge
  • [x] (GitHub staff only) Integration tests pass at github/github (Learn more about how to run integration tests)

francinelucca avatar Sep 25 '24 17:09 francinelucca

🦋 Changeset detected

Latest commit: f7433a2b32e103e2b11a212a99d8d22ca8648ba6

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

This PR includes changesets to release 1 package
Name Type
@primer/react 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

changeset-bot[bot] avatar Sep 25 '24 17:09 changeset-bot[bot]

size-limit report 📦

Path Size
packages/react/dist/browser.esm.js 97.31 KB (-0.31% 🔽)
packages/react/dist/browser.umd.js 97.51 KB (-0.29% 🔽)

github-actions[bot] avatar Sep 25 '24 17:09 github-actions[bot]

:wave: Hi, this pull request contains changes to the source code that github/github depends on. If you are GitHub staff, we recommend testing these changes with github/github using the integration workflow. Thanks!

github-actions[bot] avatar Oct 08 '24 20:10 github-actions[bot]

:wave: Hi from github/github! Your integration PR is ready: https://github.com/github/github/pull/346259

primer-integration[bot] avatar Oct 08 '24 21:10 primer-integration[bot]

🟢 golden-jobs completed with status success.

primer-integration[bot] avatar Oct 08 '24 21:10 primer-integration[bot]