adds resolved color scheme to theme context #1679
Conversation
🦋 Changeset detectedLatest commit: 2b96351 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
mattcosta7
force-pushed
the
add-resolved-color-scheme
branch
from
30afeb6
to
29af9d2
Compare
|
Something strange is going on with the package-lock.json 🤔 Probably an npm version issue |
I saw that too - what version lock/npm are y'all running, I can try to switch |
|
I'm running node 16, npm 8 |
ahh cool. the codespace I opened was 14 and 6. The nvmrc says 14. I'll upgrade and see if that helps |
|
Oh, would you mind adding a test for this in https://github.com/primer/react/blob/main/src/__tests__/ThemeProvider.test.tsx? |
Sure thing, I didn't see one for resolvedColorMode, so I let it slide, but will add. I think we should also probably read the outer scope, an pass down the resolvedColorScheme to children themes too, which I believe won't get that returned from applyColorSchemes |
Just noticed that too. Oops 😅
Not sure I understand. Could you elaborate? I think the logic for nested themes still works with this change. |
Ahh nevermind on this. I misread the deepmerge when generating the 'resolvedTheme' and thought we wouldn't have the 'colorSchemes' object for them, but we do, so all good :-D |
Done. Added a handful of cases, let me know if there's another specific case that would be worth testing |
| return <Text data-testid="text">{resolvedColorScheme}</Text> | ||
| } | ||
|
|
||
| const schemeToApply = 'totally-invalid-colorscheme' |
Head branch was pushed to by a user without write access
mattcosta7
force-pushed
the
add-resolved-color-scheme
branch
from
51da521
to
faf1292
Compare
* Add `resolvedColorScheme` property to the object returned by `useTheme() * Add `resolvedColorScheme` property to the object returned by `useTheme() * Update .changeset/red-bottles-prove.md Co-authored-by: Cole Bemis <colebemis@github.com>
Describe your changes here.
Closes #1664
This just adds the 'colorscheme' that was selected in the theme provider to the context
Screenshots
No visual change
Merge checklist
Take a look at the What we look for in reviews section of the contributing guidelines for more information on how we review PRs.