Skip to content
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.

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

chore: Radio Styles Refactor #2546

Merged

Conversation

josh-bagwell
Copy link
Contributor

Summary

Refactors styles in Radio and RadioGroup using new style utilities.

Fixes: #2396

Release Category

Components

Release Note

Radio and RadioGroup now use Canvas Tokens and our new styling utilities. The component now supports the cs prop, but otherwise the API has not changed. It should behave identically as it did in previous versions however, there may be some slight visual changes.

BREAKING CHANGES

There may be slight visual changes.


For the Reviewer

  • PR title is short and descriptive
  • PR summary describes the change (Fixes/Resolves linked correctly)
  • PR Release Notes describes additional information useful to call out in a release message or removed if not applicable
  • Breaking Changes provides useful information to upgrade to this code or removed if not applicable

Where Should the Reviewer Start?

/modules/preview-react/radio/lib/RadioGroup.tsx
/modules/preview-react/radio/lib/Radio.tsx

a smiling Shiba Inu typing on a laptop

Copy link

cypress bot commented Feb 1, 2024

Passing run #6805 ↗︎

0 905 3 0 Flakiness 0
⚠️ You've recorded test results over your free plan limit.
Upgrade your plan to view test results.

Details:

Merge 68a3298 into 61dca4f...
Project: canvas-kit Commit: d84fe80c6b ℹ️
Status: Passed Duration: 15:51 💡
Started: Feb 5, 2024 7:25 PM Ended: Feb 5, 2024 7:41 PM

Review all test suite changes for PR #2546 ↗︎

@josh-bagwell josh-bagwell marked this pull request as ready for review February 1, 2024 20:53
@josh-bagwell
Copy link
Contributor Author

UI Tests show the update to the disabled border color (from soap600 to licorice100) and the update to the alert ring for themed option.

@mannycarrera4 mannycarrera4 changed the title feat: Radio Styles Refactor chore: Radio Styles Refactor Feb 2, 2024
@josh-bagwell josh-bagwell added the ready for review Code is ready for review label Feb 2, 2024
@josh-bagwell josh-bagwell linked an issue Feb 2, 2024 that may be closed by this pull request
@alanbsmith alanbsmith requested review from alanbsmith and removed request for NicholasBoll February 2, 2024 18:58
Copy link
Member

@alanbsmith alanbsmith left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a good start. Thanks for working on this, @josh-bagwell! A few main notes:

  • If the component is already supporting style props, use mergeStyles instead of handleCSProp for backwards compatibility.
  • Let's go ahead and move everything to rem while we're refactoring
  • Use the calc functions – I made them for you. 😄
  • Maybe consolidate common styles

modules/docs/mdx/11.0-UPGRADE-GUIDE.mdx Show resolved Hide resolved
modules/preview-react/radio/lib/RadioGroup.tsx Outdated Show resolved Hide resolved
modules/preview-react/radio/lib/RadioGroup.tsx Outdated Show resolved Hide resolved
modules/preview-react/radio/lib/RadioGroup.tsx Outdated Show resolved Hide resolved
modules/preview-react/radio/lib/RadioGroup.tsx Outdated Show resolved Hide resolved
modules/preview-react/radio/lib/StyledRadioButton.tsx Outdated Show resolved Hide resolved
modules/preview-react/radio/lib/RadioText.tsx Outdated Show resolved Hide resolved
modules/preview-react/radio/lib/StyledRadioButton.tsx Outdated Show resolved Hide resolved
modules/preview-react/radio/lib/StyledRadioButton.tsx Outdated Show resolved Hide resolved
@alanbsmith alanbsmith added approved Code has been reviewed and approved (ship it) 11.x and removed ready for review Code is ready for review labels Feb 5, 2024
@alanbsmith alanbsmith merged commit a38c87c into Workday:prerelease/major Feb 5, 2024
21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
11.x approved Code has been reviewed and approved (ship it) automerge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Radio Styles Refactor
3 participants