New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix(Checkbox): react/wc parity #18329

Merged

guidari merged 10 commits into carbon-design-system:main from anamikaanu96:checkbox-parity

Feb 12, 2025

Contributor

anamikaanu96 commented Jan 13, 2025

Closes #17927

Checkbox - react and wc parity

Changelog

New

packages/web-components/src/components/checkbox/defs.ts

Changed

-packages/web-components/src/components/checkbox/checkbox-group.ts
-packages/web-components/src/components/checkbox/checkbox.scss
-packages/web-components/src/components/checkbox/checkbox.stories.ts

Testing / Reviewing

Added orientation prop and corresponding style.
Tested using storybook , can be checked in storybook controls.

anamikaanu96 added 2 commits

January 13, 2025 12:04


          fix(checkbox-parity): react/wc parity

6628bcd


          fix(checkbox-parity): resolved style issue

23821c6

anamikaanu96 requested a review from a team as a code owner

January 13, 2025 14:55

anamikaanu96 requested review from kennylam and guidari

January 13, 2025 14:55


          Merge branch 'carbon-design-system:main' into checkbox-parity

5047e10

netlify bot commented Jan 13, 2025

✅ Deploy Preview for v11-carbon-web-components ready!

Name	Link
🔨 Latest commit	`23821c6`
🔍 Latest deploy log	https://app.netlify.com/sites/v11-carbon-web-components/deploys/67852965b2b9f30008a67f3e
😎 Deploy Preview	https://deploy-preview-18329--v11-carbon-web-components.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

netlify bot commented Jan 13, 2025

✅ Deploy Preview for v11-carbon-react ready!

Built without sensitive environment variables

Name	Link
🔨 Latest commit	`23821c6`
🔍 Latest deploy log	https://app.netlify.com/sites/v11-carbon-react/deploys/67852966e8aacc0008f73733
😎 Deploy Preview	https://deploy-preview-18329--v11-carbon-react.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

netlify bot commented Jan 13, 2025

✅ Deploy Preview for carbon-elements ready!

Name	Link
🔨 Latest commit	`23821c6`
🔍 Latest deploy log	https://app.netlify.com/sites/carbon-elements/deploys/67852965d6ca6300089115be
😎 Deploy Preview	https://deploy-preview-18329--carbon-elements.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

netlify bot commented Jan 13, 2025 •

edited

Loading

✅ Deploy Preview for v11-carbon-web-components ready!

Name	Link
🔨 Latest commit	`07a2b9b`
🔍 Latest deploy log	https://app.netlify.com/sites/v11-carbon-web-components/deploys/67ab45da27ed73000955cd53
😎 Deploy Preview	https://deploy-preview-18329--v11-carbon-web-components.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

netlify bot commented Jan 13, 2025 •

edited

Loading

✅ Deploy Preview for carbon-elements ready!

Name	Link
🔨 Latest commit	`07a2b9b`
🔍 Latest deploy log	https://app.netlify.com/sites/carbon-elements/deploys/67ab45dafc021a00081a59b5
😎 Deploy Preview	https://deploy-preview-18329--carbon-elements.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

netlify bot commented Jan 13, 2025 •

edited

Loading

✅ Deploy Preview for v11-carbon-react ready!

Built without sensitive environment variables

Name	Link
🔨 Latest commit	`07a2b9b`
🔍 Latest deploy log	https://app.netlify.com/sites/v11-carbon-react/deploys/67ab45dacf8a5c0008067950
😎 Deploy Preview	https://deploy-preview-18329--v11-carbon-react.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

codecov bot commented Jan 13, 2025 •

edited

Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 84.01%. Comparing base (bc04c97) to head (07a2b9b).
Report is 23 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #18329      +/-   ##
==========================================
- Coverage   84.16%   84.01%   -0.16%     
==========================================
  Files         408      408              
  Lines       14450    14477      +27     
  Branches     4666     4664       -2     
==========================================
+ Hits        12162    12163       +1     
- Misses       2123     2147      +24     
- Partials      165      167       +2

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

anamikaanu96 changed the title ~~Checkbox parity~~ fix(Checkbox): react/wc parity


          Merge branch 'carbon-design-system:main' into checkbox-parity

bfa4b49

davidmenendez suggested changes

View reviewed changes

Contributor

davidmenendez left a comment

looks good. one minor nitpick. as a convention we should continue to stick with constants.{ts,js,etc} instead of defs

tay1orjones added the status: visual review 🔍 label

tay1orjones requested review from a team and thyhmdo and removed request for a team

January 14, 2025 14:26

kennylam requested changes

View reviewed changes

Member

kennylam left a comment

Looks great! Just a few small changes.

packages/web-components/src/components/checkbox/checkbox-group.ts Outdated Show resolved Hide resolved

packages/web-components/src/components/checkbox/checkbox.scss Outdated Show resolved Hide resolved

packages/web-components/src/components/checkbox/checkbox.stories.ts Outdated Show resolved Hide resolved

packages/web-components/src/components/checkbox/checkbox.stories.ts Outdated Show resolved Hide resolved

packages/web-components/src/components/checkbox/checkbox.stories.ts Outdated Show resolved Hide resolved

Member

tay1orjones commented Jan 15, 2025

Hey, thanks for putting up this PR! I just added @thyhmdo as a reviewer to be sure we get a ux and visual review on this.


          fix(checkbox): parity review changes

04f2fa3

Contributor Author

anamikaanu96 commented Jan 16, 2025

looks good. one minor nitpick. as a convention we should continue to stick with constants.{ts,js,etc} instead of defs

Hey @davidmenendez Thanks for reviewing! In Carbon Web Components, I’ve noticed that they use the naming convention def.ts for constants.

anamikaanu96 requested a review from kennylam

January 16, 2025 05:49


          Merge branch 'main' into checkbox-parity

27f631e

guidari requested changes

View reviewed changes

Contributor

guidari left a comment

Hey @anamikaanu96
LGTM, just one minor thing. We should remove the Playground stories and add the controls to the Default stories, to match React storybook.


          fix(checkbox): playground changes

d0ba93b

anamikaanu96 requested review from guidari and davidmenendez

February 3, 2025 07:34


          Merge branch 'main' into checkbox-parity

777b5a8

guidari approved these changes

View reviewed changes

Contributor

guidari left a comment

LGTM!

kennylam approved these changes

View reviewed changes

Member

ariellalgilmore commented Feb 5, 2025 •

edited

Loading

Hi @anamikaanu96, an issue was brought up in slack. https://ibm-studios.slack.com/archives/CL83LMKSA/p1738774002789579. Looks like some accessibility errors are happening in web components and not in react. We could open a seperate issue for this or fix it here


          fix(checkbox-parity): accessibility errors

d7ae970

ariellalgilmore approved these changes

View reviewed changes

Member

thyhmdo commented Feb 10, 2025

Web components Single controls seem to be missing out on a few props like defaultChecked and indeterminate?


          fix(checkbox): review changes

07a2b9b

anamikaanu96 requested review from a team as code owners

February 11, 2025 12:43

anamikaanu96 requested review from preetibansalui and tay1orjones

February 11, 2025 12:43

thyhmdo approved these changes

View reviewed changes

Member

thyhmdo left a comment

looks great!

guidari added this pull request to the merge queue

guidari added the status: ready to merge 🎉 label

Merged via the queue into carbon-design-system:main with commit 2fbb4a9

37 checks passed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

kennylam kennylam approved these changes

ariellalgilmore ariellalgilmore approved these changes

guidari guidari approved these changes

thyhmdo thyhmdo approved these changes

davidmenendez Awaiting requested review from davidmenendez

preetibansalui Awaiting requested review from preetibansalui preetibansalui is a code owner automatically assigned from carbon-design-system/monorepo-reviewers

tay1orjones Awaiting requested review from tay1orjones tay1orjones is a code owner automatically assigned from carbon-design-system/monorepo-lead-reviewers

Labels

status: ready to merge 🎉 status: visual review 🔍