Skip to content
This repository has been archived by the owner on Jun 26, 2020. It is now read-only.

Refactor theme to use CSS variables #1090

Closed
bvaughn opened this issue Aug 10, 2018 · 4 comments
Closed

Refactor theme to use CSS variables #1090

bvaughn opened this issue Aug 10, 2018 · 4 comments

Comments

@bvaughn
Copy link
Contributor

bvaughn commented Aug 10, 2018

We currently use context to pass around the active Theme and is very heavy-handed. It forces use to use factory functions for otherwise static CSS styles and to force-deep-render the app when the current theme changes.

In hindsight, I should have used CSS variables for this. Fortunately, it should be easy to gradually migrate components over to using CSS variables instead of context. All we need to do is update the variables whenever a theme is loaded, and then we can gradually remove context references in favor of CSS variables over time.

@infinnie
Copy link

What about the new context API combined with styled components?

@bvaughn
Copy link
Contributor Author

bvaughn commented Sep 22, 2018

I don't think context is necessary. CSS variables would let us do this in a way that totally avoids the problem of re-rendering. It also simplifies components as they no longer need to worry about a theme param.

I'm not convinced that styled-components buys us anything that plain styles + CSS doesn't, and at least it used to be that the lib came with a significant performance cost. The performance thing might have changed. I've heard that the latest release made significant performance gains. But I don't personally find the API compelling in the case of DevTools.

Maybe you could put together a prototype that shows how you'd use SC in a specific DevTools panel– maybe showing a side by side comparison of inline styles versus SC– and make an argument for it?

@bvaughn
Copy link
Contributor Author

bvaughn commented Nov 21, 2018

PR #1236 updates our infra to support CSS modules. They aren't strictly necessary for this change– (we could use CSS variables with inline styles)– but I think the combination of the two really cuts down on a lot of boilerplate and "feels" better.

@bvaughn
Copy link
Contributor Author

bvaughn commented Aug 19, 2019

React DevTools has been rewritten and recently launched a new version 4 UI. The source code for this rewrite was done in a separate repository and now lives in the main React repo (github.com/facebook/react).

Because version 4 was a total rewrite, and all issues in this repository are related to the old version 3 of the extension, I am closing all issues in this repository. If you can still reproduce this issue, or believe this feature request is still relevant, please open a new issue in the React repo: https://github.com/facebook/react/issues/new?labels=Component:%20Developer%20Tools

@bvaughn bvaughn closed this as completed Aug 19, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants