Repository navigation
[DevTools] Refactor imperative theme code #21900
Description
DevTools defaults a "light" and a "dark" theme in a root-level CSS file:
https://github.com/facebook/react/blob/310187264d01a31bc3079358f13662d31a079d9e/packages/react-devtools-shared/src/devtools/views/root.css#L1-L188
The current theme value ("light", "dark", or "auto") is stored in the ThemeContext and persisted between sessions using the localStorage API:
https://github.com/facebook/react/blob/310187264d01a31bc3079358f13662d31a079d9e/packages/react-devtools-shared/src/devtools/views/Settings/SettingsContext.js#L86-L89
When the theme changes, a layout effect updates root-level CSS variables:
https://github.com/facebook/react/blob/310187264d01a31bc3079358f13662d31a079d9e/packages/react-devtools-shared/src/devtools/views/Settings/SettingsContext.js#L149-L163
This has an unfortunate side effect of creating a race condition between any other imperative code (e.g. a Canvas rendering component) that may want to read the current CSS variables. This code cannot run before the code above or it will read previous/stale values.
We should refactor this code to declaratively share theme variables to its subtree, e.g.:
function ThemeProvider({ value, children }) {
// ...
return (
<ThemeContext.Provider value={theme}>
<div
style={{
'--color-attribute-name': '#ef6632',
'--color-attribute-name-not-editable': '#23272f',
// ...
}}
>
{children}
</div>
</ThemeContext.Provider>
);
}We will also need to update any place that reads these computed styles, e.g.
https://github.com/facebook/react/blob/310187264d01a31bc3079358f13662d31a079d9e/packages/react-devtools-shared/src/devtools/views/Settings/SettingsContext.js#L274-L281
A similar approach is used for font size ("comfortable" or "compact"). We should do the same refactor for this.
Activity
houssemchebeb commentedon Jul 16, 2021
Hi @bvaughn, can i take this ?
bvaughn commentedon Jul 17, 2021
This issue is all yours @houssemchebeb 😄
I've added the "good first issue (taken)" label so that others will know not to start work on the issue. If you change your mind about the issue, no worries! Just let me know so that I can remove the label and free it up for someone else to claim.
Cheers!
houssemchebeb commentedon Jul 17, 2021
Thanks :)
NishantChandla commentedon Jul 23, 2021
@houssemchebeb Is this issue up for grabs? Do you mind if i take over
houssemchebeb commentedon Jul 23, 2021
@NishantChandla I am already working on this issue sorry