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 Label API #934

Merged
merged 15 commits into from
Aug 5, 2022
Merged

Refactor Label API #934

merged 15 commits into from
Aug 5, 2022

Conversation

pouretrebelle
Copy link
Member

@pouretrebelle pouretrebelle commented Dec 6, 2021 •
edited
Loading

Description

Updating the Label API without any need for CSS changes

  • Replace variant: :large with size: :large

Consequences

  • Global replace of variant: :large with size: :large on gh/gh

@vercel
Copy link

vercel bot commented Dec 6, 2021 •
edited
Loading

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/primer/view-components/LTs4fVobQ1HEpjgyjtbg18hFUF9t
✅ Preview: https://view-components-git-pouretrebelle-label-api-primer.vercel.app

@vercel vercel bot temporarily deployed to Preview December 6, 2021 08:58 Inactive
@vercel vercel bot temporarily deployed to Preview December 6, 2021 09:05 Inactive
@vercel vercel bot temporarily deployed to Preview December 6, 2021 09:06 Inactive
@vercel vercel bot temporarily deployed to Preview December 6, 2021 11:42 Inactive
@vercel vercel bot temporarily deployed to Preview December 6, 2021 12:22 Inactive
@vercel vercel bot temporarily deployed to Preview December 6, 2021 12:34 Inactive
@vercel vercel bot temporarily deployed to Preview December 6, 2021 12:37 Inactive
@pouretrebelle
Extract size from variant Label argument
5778611 
To standardise the API for labels we want to create a consistent size
arg. The inline variant effects more CSS than the display property so
I've left is as a variant, with intention to either deprecate or
isolate the display functionality and rename it.
@vercel vercel bot temporarily deployed to Preview December 6, 2021 14:03 Inactive
@pouretrebelle pouretrebelle marked this pull request as ready for review December 6, 2021 16:12
@pouretrebelle pouretrebelle requested review from a team and manuelpuyol December 6, 2021 16:12
manuelpuyol
manuelpuyol reviewed Dec 6, 2021
View reviewed changes
Copy link
Contributor

@manuelpuyol manuelpuyol left a comment

Choose a reason for hiding this comment

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

could you add a CHANGELOG entry? :)

app/components/primer/label_component.rb Outdated Show resolved Hide resolved
app/components/primer/label_component.rb Outdated Show resolved Hide resolved
@github-actions
Copy link
Contributor

Hi! This pull request has been marked as stale because it has been open with no activity for 60 days. You can comment on the pull request or remove the stale label to keep it open. If you do nothing, this pull request will be closed in 7 days.

@github-actions github-actions bot added the Stale Automatically marked as stale. label May 24, 2022
@joelhawksley joelhawksley removed the Stale Automatically marked as stale. label May 25, 2022
@github-actions
Copy link
Contributor

Hi! This pull request has been marked as stale because it has been open with no activity for 60 days. You can comment on the pull request or remove the stale label to keep it open. If you do nothing, this pull request will be closed in 7 days.

@github-actions github-actions bot added the Stale Automatically marked as stale. label Jul 24, 2022
@lesliecdubs lesliecdubs requested review from a team and jonrohan July 29, 2022 22:44
@lesliecdubs lesliecdubs removed the Stale Automatically marked as stale. label Jul 29, 2022
@lesliecdubs
Copy link
Member

Added Rails reviewers on this as it's gone a bit stale. I realize there are quite a few conflicts on this now, but with @pouretrebelle out would love to get someone else's eyes on to see if this is something we should revive or close. Thanks!

@camertron
Copy link
Contributor

@lesliecdubs I'm happy to help get this across the finish line 😄

@changeset-bot
Copy link

changeset-bot bot commented Aug 1, 2022 •
edited
Loading

🦋 Changeset detected

Latest commit: 4f1f679

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

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

@primer-css primer-css temporarily deployed to github-pages August 1, 2022 18:48 Inactive
@camertron camertron temporarily deployed to github-pages August 1, 2022 19:36 Inactive
@primer-css primer-css temporarily deployed to github-pages August 1, 2022 20:31 Inactive
@camertron camertron temporarily deployed to github-pages August 1, 2022 20:40 Inactive
@camertron camertron temporarily deployed to github-pages August 1, 2022 22:17 Inactive
@camertron
Copy link
Contributor

@jonrohan @manuelpuyol I've adopted this branch. It's ready for another look 👀

@camertron camertron temporarily deployed to github-pages August 1, 2022 22:43 Inactive
@jonrohan jonrohan merged commit d638fef into main Aug 5, 2022
@jonrohan jonrohan deleted the pouretrebelle/label-api branch August 5, 2022 18:21
@primer-css primer-css mentioned this pull request Aug 5, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jonrohan jonrohan jonrohan approved these changes

@manuelpuyol manuelpuyol Awaiting requested review from manuelpuyol

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@pouretrebelle @lesliecdubs @camertron @jonrohan @manuelpuyol @joelhawksley @primer-css @actions-user