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

Button API alignment #2714

Closed
wants to merge 6 commits into from
Closed

Button API alignment #2714

wants to merge 6 commits into from

Conversation

langermank
Copy link
Contributor

@langermank langermank commented Dec 20, 2022

This PR includes breaking changes to bring the PRC Button up to the agreed upon API spec and match implementation with PVC.

  • Changes leadingIcon and trailingIcon to leadingVisual and trailingVisual (a pattern introduced with ActionList)
  • Removes Button.Counter as a child component, replacing it with a trailingVisualCount prop. This change allows us to use the trailingVisual slot for counters, and also aligns better with all the other child elements within Button (visuals are props, not child components in this case.)
  • Removes the outline button variant as we wish to only support invisible buttons.

Screenshots

The only visual change is that the outline button variant has been removed.

Merge checklist

  • Added/updated tests
  • Added/updated documentation
  • Tested in Chrome
  • Tested in Firefox
  • Tested in Safari
  • Tested in Edge

@langermank langermank added the major release breaking changes label Dec 20, 2022
@langermank langermank requested review from a team and JoshBowdenConcepts December 20, 2022 19:29
@changeset-bot
Copy link

changeset-bot bot commented Dec 20, 2022

🦋 Changeset detected

Latest commit: 674e54d

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 Major

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

@github-actions
Copy link
Contributor

github-actions bot commented Dec 20, 2022

size-limit report 📦

Path Size
dist/browser.esm.js 84.75 KB (-0.19% 🔽)
dist/browser.umd.js 85.35 KB (-0.2% 🔽)

@github-actions github-actions bot temporarily deployed to storybook-preview-2714 December 20, 2022 19:35 Inactive
@github-actions github-actions bot temporarily deployed to storybook-preview-2714 December 20, 2022 19:38 Inactive
@langermank langermank temporarily deployed to github-pages December 20, 2022 19:42 — with GitHub Actions Inactive
@github-actions github-actions bot temporarily deployed to storybook-preview-2714 December 20, 2022 19:42 Inactive
@joshblack
Copy link
Member

Hey @langermank! I've been trying to make sure all the branches are up-to-date with the new vrt workflow. I wanted to ask if you have any free time this week to update the conflicts on this PR?

@langermank
Copy link
Contributor Author

@joshblack thanks for the reminder! I'm waiting for this PR that @broccolinisoup is working with me on to land before I update this PR since I think it will resolve a lot of the conflicts here. I had to revert the original PR that this PR is based on.

@langermank langermank marked this pull request as draft January 25, 2023 17:18
@langermank
Copy link
Contributor Author

Will reopen once #2792 merges

@langermank langermank closed this Jan 31, 2023
@langermank
Copy link
Contributor Author

langermank commented Jan 31, 2023

Note to self: add these changes to this PR d79fbfc

@joshblack joshblack deleted the button-api-alignment branch January 31, 2023 22:53
@langermank langermank restored the button-api-alignment branch February 10, 2023 21:43
@siddharthkp siddharthkp deleted the button-api-alignment branch March 27, 2023 11:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
major release breaking changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants