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

refactor(one-dark): export all theme colors for reuse is needed #3

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

bRRRITSCOLD
Copy link

Export theme colors for reuse by down stream users to match said theme colors/reuse of said theme colors

@bRRRITSCOLD
Copy link
Author

@marijnh If you have some time - I just want to reuse the theme color for some things in an app, don't want to use a forked version or keep local colors up to date with this repo

@marijnh
Copy link
Member

marijnh commented Aug 4, 2022

  1. We're definitely not breaking backwards compat or bumping the major version of any of these packages.
  2. This looks entirely broken and untested so it's really hard to take it seriously.

@bRRRITSCOLD
Copy link
Author

  1. @marijnh how does this break backwards compat? The things that are exported and renamed now were never exported to begin with, so no one would be using them in their code, therefore there is no possible way we break anyone.
  2. I will gladly update tests now! :)

@bRRRITSCOLD
Copy link
Author

bRRRITSCOLD commented Aug 4, 2022

  1. @marijnh you have no tests.... It works completely fine for me locally, so for "tested" idk what more your repo requires, your whole codebase is "untested" isn't it?

@marijnh
Copy link
Member

marijnh commented Aug 4, 2022

how does this break backwards compat?

You're the one who moved it to version 7.0.0.

I'm not talking about automated tests. I'm talking about the fact that you're using CSS properties like one-dark-background-color, which are definitely not something the browser will use.

@bRRRITSCOLD
Copy link
Author

bRRRITSCOLD commented Aug 4, 2022

backgroundColor to oneDarkBackgroundColor is clearly a mistake on my end.... Its an obvious "ctrl + f" mistake, fixing now.

As for 7.0.0 would your suggestion to be just a minor bump? 6.1.0? 6.0.1?

marijnh added a commit that referenced this pull request Aug 4, 2022
FEATURE: Export a `color` object holding the colors used in the
theme.

Issue #3
@marijnh
Copy link
Member

marijnh commented Aug 4, 2022

Would attached patch work as a replacement for this?

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.

2 participants