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.

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

refactor!: rebrand Chip & ChipGroup components #1948

Merged
merged 14 commits into from
Jan 16, 2024
Merged

refactor!: rebrand Chip & ChipGroup components #1948

merged 14 commits into from
Jan 16, 2024

Conversation

snitin315
Copy link
Member

@snitin315 snitin315 commented Jan 10, 2024 •
edited
Loading

  • Removed intent prop.
    • intent="none|positive|negative" -> color="primary|positive|negative"
  • Changed color="default" to color="primary".
  • Streamlined and refreshed tokens for a sleeker look.
  • Updated codemod to handle breaking changes.
  • JIRA Link

@changeset-bot changeset-bot
Copy link

changeset-bot bot commented Jan 10, 2024 •
edited
Loading

⚠️ No Changeset found

Latest commit: 24625ea

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

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

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Jan 10, 2024 •
edited
Loading

✅ PR title follows Conventional Commits specification.

@snitin315 snitin315 marked this pull request as draft January 10, 2024 06:09
@codesandbox-ci CodeSandbox CI
Copy link

codesandbox-ci bot commented Jan 10, 2024 •
edited
Loading

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 24625ea:

Sandbox Source
razorpay/blade: basic Configuration

@snitin315 snitin315 marked this pull request as ready for review January 10, 2024 06:15
@snitin315 snitin315 added the Review - L1 First level of review label Jan 10, 2024
@snitin315
Copy link
Member Author

Fixing types...

anuraghazra
anuraghazra approved these changes Jan 12, 2024
View reviewed changes
Copy link
Member

@anuraghazra anuraghazra left a comment

Choose a reason for hiding this comment

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

LGTM

@snitin315 snitin315 added Review - L2 Second level of review and removed Review - L1 First level of review labels Jan 12, 2024
kamleshchandnani
kamleshchandnani reviewed Jan 15, 2024
View reviewed changes
packages/blade/src/components/Chip/AnimatedChip.native.tsx
@@ -22,8 +22,8 @@ const AnimatedChip = ({
}: Omit<AnimatedChipProps, 'theme'>): React.ReactElement => {
const { theme } = useTheme();

const easing = getIn(theme, chipMotionTokens.easing);
const duration = getIn(theme, chipMotionTokens.duration);
const easing = getIn(theme.motion, chipMotionTokens.easing);
Copy link
Collaborator

Choose a reason for hiding this comment

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

was this broken earlier?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, just refactored types as per #1948 (comment)

kamleshchandnani
kamleshchandnani reviewed Jan 15, 2024
View reviewed changes
packages/blade/src/components/Chip/__tests__/Chip.web.test.tsx
@@ -39,22 +39,6 @@ describe('<Chip />', () => {
expect(getByRole('radio', { name: 'Mango' })).toBeInTheDocument();
});

it('should render chip with intent', () => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

this shall ideally fail right? instead of removing this test maybe check for failure?

Copy link
Member Author

Choose a reason for hiding this comment

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

TS will throw an error. The component won't throw any runtime error because we don't have any runtime checks. We have removed intent/variant tests in other components too.

@kamleshchandnani kamleshchandnani merged commit 5fc8ed0 into rebranding/master Jan 16, 2024
12 of 14 checks passed
@kamleshchandnani kamleshchandnani deleted the chip-rebrand branch January 16, 2024 06:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@anuraghazra anuraghazra anuraghazra approved these changes

@kamleshchandnani kamleshchandnani kamleshchandnani approved these changes

@chaitanyadeorukhkar chaitanyadeorukhkar Awaiting requested review from chaitanyadeorukhkar

Assignees

@kamleshchandnani kamleshchandnani

Labels
Review - L2 Second level of review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@snitin315 @kamleshchandnani @anuraghazra @chaitanyadeorukhkar