feat(graphiql-react): add useTheme to context #2971
Conversation
🦋 Changeset detectedLatest commit: f0ac054 The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
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 |
|
|
|
It would be great to get the return values updated as well, see suggestion here. Also, would love to see some tests for this new functionality. |
matijagaspar
force-pushed
the
feat(graphiql-react)/add-theme-to-context
branch
from
1a0ba35 to
fc92aae
Compare
| ); | ||
| } | ||
|
|
||
| export const useTheme = createContextHook(ThemeContext); |
There was a problem hiding this comment.
can you add a documentation block above, for the api docs? annotations will automatically be inferred, just for the description
matijagaspar
force-pushed
the
feat(graphiql-react)/add-theme-to-context
branch
from
fc92aae to
e85399a
Compare
matijagaspar
force-pushed
the
feat(graphiql-react)/add-theme-to-context
branch
from
e85399a to
f0ac054
Compare
|
@thomasheyenbrock and @jonathanawesome - should we ask for tests for this? |
There was a problem hiding this comment.
Thanks a lot @matijagaspar, that's an awesome addition!
Starting to add some tests to this would be awesome, but I wouldn't put that on @matijagaspar - unless you're up to it!
| type ThemeContextProviderProps = { children: ReactNode }; | ||
|
|
||
| export function ThemeContextProvider(props: ThemeContextProviderProps) { | ||
| const theme = useThemeInternal(); |
There was a problem hiding this comment.
Nit: We could just inline the contents from that hook here and avoid the extra function.
Fixes #2956
Makes
useThemea context instead of local state so it can be used in plugins (or any children of<GraphiQL/>) when using:const {theme, setTheme} = useTheme()