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

Remove border-overlay #82

Closed
wants to merge 4 commits into from
Closed

Remove border-overlay #82

wants to merge 4 commits into from

Conversation

simurai
Copy link
Contributor

@simurai simurai commented Apr 27, 2021

Closes https://github.com/github/design-systems/issues/1393.

  • 🔥 Remove border-overlay.

⚠️ Note: This is a breaking change. It's not too urgent to be released. Most Primer repos already have migration PRs ready. See below.

Migration guide

The border-overlay can be replaced with border-primary. There should be no visual difference since both variables use the same color.

@changeset-bot
Copy link

changeset-bot bot commented Apr 27, 2021

🦋 Changeset detected

Latest commit: 184919c

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

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

@vercel
Copy link

vercel bot commented Apr 27, 2021

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/primitives/EsUcohry8PDgSeohb2quHUSrKzq6
✅ Preview: https://primitives-git-overlay-border-primer.vercel.app

@auareyou
Copy link
Contributor

My suggestion was to remove border-overlay - It was never part of the system. Is that doable?

@simurai simurai marked this pull request as draft April 27, 2021 09:09
@simurai simurai changed the title Alias border-overlay with border-primary Remove border-overlay Apr 27, 2021
@simurai
Copy link
Contributor Author

simurai commented Apr 28, 2021

Is that doable?

Yes, doable. Just needs a few more changes in other Primer repos and on dotcom.

Also, tried to figure out if we need to remove it from Figma? I found it in this Figma file. And it's also available from the color picker in the sidebar:

Screen Shot 2021-04-28 at 17 30 04

Not sure how/where to replace border/overlay with border/primary so that it syncs with all other Figma files that use that color. 🤔

/cc @ashygee

@auareyou
Copy link
Contributor

Do you know where this one is used outside of Primer?

@simurai
Copy link
Contributor Author

simurai commented Apr 29, 2021

Do you know where this one is used outside of Primer?

Hard to say. We currently only keep an eye on Figma, Primer repos and dotcom. Other apps/sites will have to change it themselves when they wanna update to a newer version of Primer.

@simurai simurai marked this pull request as ready for review April 30, 2021 01:14
@ashygee
Copy link

ashygee commented May 3, 2021

Not sure how/where to replace border/overlay with border/primary so that it syncs with all other Figma files that use that color. 🤔

@simurai this would be a simple find/replace in Figma. I can show you how I do it for future updates/removals too.

@simurai
Copy link
Contributor Author

simurai commented May 27, 2021

Let's close this again since in V2 border-overlay is mapped to border.default. So it will be removed anyways.

@simurai simurai closed this May 27, 2021
@simurai simurai deleted the overlay-border branch May 27, 2021 00:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants