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

Update marketing buttons (including color mode support) #1716

Merged
merged 16 commits into from
Nov 10, 2021

Conversation

tobiasahlin
Copy link
Contributor

@tobiasahlin tobiasahlin commented Oct 29, 2021

This PR upstreams the latest changes to marketing buttons from github.com, which includes proper support for color modes. Preview:

buttons-draft.mov

primer/primitives#265 is intended to land first (primer/[email protected]), at which point this PR can be updated to use those variables instead of local overrides.

/cc @primer/css-reviewers

@changeset-bot
Copy link

changeset-bot bot commented Oct 29, 2021

🦋 Changeset detected

Latest commit: 2a84afa

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

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

Copy link
Contributor

@simurai simurai left a comment

Choose a reason for hiding this comment

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

So far looks good. ✨

Are the !important needed to override utilities?

Copy link
Contributor

@simurai simurai left a comment

Choose a reason for hiding this comment

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

Ok, did the following:

  1. Updated primer/primitives to 7.0.1 that includes the new mktg variables e372b03
  2. Replace the temp variables with primer/primitives variables e258ff5

@tobiasahlin does this 👀 preview look right? Looks good to me. 👍

Maybe one question before merging: When we ship this to dotcom, will there be lots of refactoring needed? Like replace all the updated btn classes?

@tobiasahlin
Copy link
Contributor Author

@simurai amazing, thank you for pulling in those changes and replacing all the overrides! 🙏

Preview looks great 👀 I just replaced the replacement suggestion of btn-transparent → btn-muted-mktg with btn-transparent → btn-subtle-mktg

Maybe one question before merging: When we ship this to dotcom, will there be lots of refactoring needed? Like replace all the updated btn classes?

@simurai yes a few updates, but it'll be a fairly light lift. All of these styles are already in .com with custom overrides, so there'll be no visual changes with this update, just replacing classes and cleaning up the temporary custom overrides 🙏

@simurai
Copy link
Contributor

simurai commented Nov 10, 2021

Ok, let's 🚢 this.

@simurai simurai merged commit 9b97dc8 into main Nov 10, 2021
@simurai simurai deleted the tobiasahlin/gloss-buttons-v2 branch November 10, 2021 04:55
@primer-css primer-css mentioned this pull request Nov 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants