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

complete theme injection #1

Merged
merged 5 commits into from
Oct 1, 2020

Conversation

nandorojo
Copy link
Collaborator

@nandorojo nandorojo commented Sep 30, 2020

@hasparus is this the right approach to adding more scales here?

Note, I haven't added any comments yet, but if this is the right path, then I could do that next.

cc @cmaycumber

PS I have this set to merge into @hasparus's branch, not sure if that's the right move or not. I haven't done a PR pointed at another PR before.

@hasparus
Copy link
Owner

@hasparus is this the right approach to adding more scales here?

If it works for you. Or actually, I could move my branch to theme-ui repo to keep all conversation there.

@hasparus
Copy link
Owner

BTW. All CSS object properties are optional (:?) I noticed in the CI failure.

@hasparus
Copy link
Owner

Or actually, I could move my branch to theme-ui repo to keep all conversation there.

@nandorojo, it's impossible to change base branch of the PR. Let's keep it here, I'll try to merge anything swiftly so we can still be focused on system-ui/theme-ui repo and use this branch just to consolidate the code.

@nandorojo nandorojo changed the title add opacities, space (no comments yet) complete theme injection Oct 1, 2020
@nandorojo
Copy link
Collaborator Author

@hasparus Looks like my tests are failing in the css package still. I'm doing this a bit brute-force since I'm not super familiar with the code base, but I believe I've followed your work so far with the colors.

Does anything stand out to you as the reason for the errors here so far?

@hasparus
Copy link
Owner

hasparus commented Oct 1, 2020

I think we can merge it with failing tests. I'll take a look at them tonight.

@hasparus
Copy link
Owner

hasparus commented Oct 1, 2020

I'm giving you access to this fork so you can merge whenever you're ready.

@nandorojo
Copy link
Collaborator Author

I think we can merge it with failing tests. I'll take a look at them tonight.

Appreciate that, thanks!

@hasparus hasparus merged commit 1ccc26b into hasparus:inject-exact-theme Oct 1, 2020
hasparus pushed a commit that referenced this pull request Feb 8, 2021
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