Skip to content

feat(Token): Migrate to CSS modules behind feature flag Pt 1 - #5251

Merged
randall-krauskopf merged 10 commits into
mainfrom
token-css-modules-1
Nov 12, 2024
Merged

feat(Token): Migrate to CSS modules behind feature flag Pt 1#5251
randall-krauskopf merged 10 commits into
mainfrom
token-css-modules-1

Conversation

@randall-krauskopf

@randall-krauskopfrandall-krauskopf commented Nov 8, 2024•
edited
Loading

Copy link
Copy Markdown
Contributor

Related To: https://github.com/github/primer/issues/4367

This PR is part 1 of breaking apart a larger PR into grokkable parts.

@changeset-bot

changeset-botBot commented Nov 8, 2024•
edited
Loading

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: 0628003

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-actionsBot commented Nov 8, 2024

Copy link
Copy Markdown
Contributor

👋 Hi, this pull request contains changes to the source code that github/github depends on. If you are GitHub staff, we recommend testing these changes with github/github using the integration workflow. Thanks!

@github-actionsgithub-actionsBot added the integration-tests: recommended This change needs to be tested for breaking changes. See https://arc.net/l/quote/tdmpakpm label Nov 8, 2024
@randall-krauskopfrandall-krauskopf added the skip changeset This change does not need a changelog label Nov 8, 2024
@github-actions

github-actionsBot commented Nov 8, 2024•
edited
Loading

Copy link
Copy Markdown
Contributor

size-limit report 📦

PathSize
packages/react/dist/browser.esm.js98.75 KB (+0.03% 🔺)
packages/react/dist/browser.umd.js99.18 KB (+0.12% 🔺)

@github-actions
github-actionsBottemporarily deployed to storybook-preview-5251 November 8, 2024 19:05 Inactive
@randall-krauskopfrandall-krauskopf changed the title Migrate container and remove button to CSS modulesfeat(Token): Migrate to CSS modules behind feature flag Pt 1Nov 8, 2024
@github-actions
github-actionsBottemporarily deployed to storybook-preview-5251 November 8, 2024 19:26 Inactive
@github-actions
github-actionsBottemporarily deployed to storybook-preview-5251 November 8, 2024 19:38 Inactive
@primer-integration

primer-integrationBot commented Nov 8, 2024•
edited
Loading

Copy link
Copy Markdown

🟢 golden-jobs completed with status success.

@github-actionsgithub-actionsBot added integration-tests: failing Changes in this PR cause breaking changes in gh/gh and removed integration-tests: recommended This change needs to be tested for breaking changes. See https://arc.net/l/quote/tdmpakpm labels Nov 8, 2024
@github-actions
github-actionsBottemporarily deployed to storybook-preview-5251 November 8, 2024 22:08 Inactive
@github-actions
github-actionsBottemporarily deployed to storybook-preview-5251 November 8, 2024 22:35 Inactive
@primer-integration

primer-integrationBot commented Nov 8, 2024

Copy link
Copy Markdown

👋 Hi from github/github! Your integration PR is ready: https://github.com/github/github/pull/350628

@github-actionsgithub-actionsBot added integration-tests: passing Changes in this PR do NOT cause breaking changes in gh/gh and removed integration-tests: failing Changes in this PR cause breaking changes in gh/gh labels Nov 8, 2024
@github-actions
github-actionsBottemporarily deployed to storybook-preview-5251 November 12, 2024 15:30 Inactive
@randall-krauskopf
randall-krauskopf marked this pull request as ready for review November 12, 2024 15:40
@randall-krauskopf
randall-krauskopf requested a review from a team as a code ownerNovember 12, 2024 15:40
jonrohan
jonrohan reviewed Nov 12, 2024
View reviewed changes
Comment threadpackages/react/src/Token/_RemoveTokenButton.module.css
}

.Bigger {
margin-left: var(--base-size-8);

@jonrohanjonrohanNov 12, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Curious could you add this to the .TokenButton[data-size='xlarge'] and .TokenButton[data-size='large'] selectors instead of adding .Bigger?

@randall-krauskopfrandall-krauskopfNov 12, 2024

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Good callout! Update

@github-actions
github-actionsBottemporarily deployed to storybook-preview-5251 November 12, 2024 21:06 Inactive
jonrohan
jonrohan reviewed Nov 12, 2024
View reviewed changes
Comment threadpackages/react/src/Token/_RemoveTokenButton.tsx Outdated
data-size={size}
className={clsx(
enabled && classes.TokenButton,
enabled && addLargerMarginClass(size) && classes.Bigger,

@jonrohanjonrohanNov 12, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
enabled && addLargerMarginClass(size) && classes.Bigger,

@randall-krauskopfrandall-krauskopfNov 12, 2024

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

ugh, 🤦 thank you for catching that

@randall-krauskopfrandall-krauskopfNov 12, 2024

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Removed

@github-actions
github-actionsBottemporarily deployed to storybook-preview-5251 November 12, 2024 21:15 Inactive
@github-actions
github-actionsBottemporarily deployed to storybook-preview-5251 November 12, 2024 21:56 Inactive
jonrohan
jonrohan approved these changes Nov 12, 2024
View reviewed changes

@jonrohanjonrohan left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

🤘🏻

@randall-krauskopf
randall-krauskopf added this pull request to the merge queueNov 12, 2024
Merged via the queue into main with commit 536d7abNov 12, 2024
@randall-krauskopf
randall-krauskopf deleted the token-css-modules-1 branch November 12, 2024 22:16
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jonrohanjonrohanjonrohan approved these changes
@broccolinisoupbroccolinisoupAwaiting requested review from broccolinisoupbroccolinisoup is a code owner automatically assigned from primer/engineer-reviewers
@joshblackjoshblackAwaiting requested review from joshblack

Assignees

No one assigned

Labels

integration-tests: passingChanges in this PR do NOT cause breaking changes in gh/ghskip changesetThis change does not need a changelog

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@randall-krauskopf@jonrohan@hussam-i-am