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

Consider injecting styles to style tags? #1163

Closed
infinnie opened this issue Sep 20, 2018 · 8 comments
Closed

Consider injecting styles to style tags? #1163

infinnie opened this issue Sep 20, 2018 · 8 comments

Comments

@infinnie
Copy link

infinnie commented Sep 20, 2018

Inline styles seem to have become a complete mess in React DevTools. We have to add event handlers for even hover or focused states. To make things worse, inline styles do not support pseudo-classes and pseudo-elements like :before, :after, and placeholders. Using libraries such as Styled Components to dynamically inject styles, the problem could be alleviated to a certain extent.

@infinnie infinnie changed the title Consider injecting styles? Consider injecting styles to style tags? Sep 20, 2018
@bvaughn
Copy link
Contributor

bvaughn commented Sep 22, 2018

As a maintainer, I share your opinion that the current state of inconsistent styling is a mess for DevTools, but I don't think this issue is really actionable– because it's basically saying to refactor all of the UI code for something that won't bring any benefit to React DevTools users.

If we had a much larger team, I'd love to tackle something like this. (In fact, I filed a related issue about this already– #1090.) But I don't think it's a high enough priority to get done any time soon given everything else that's going on.

If you're interested in contributing, maybe cleaning up the styles would be a good area to tackle?

For what it's worth, I don't agree with the idea of eliminating all inline styles in favor of external style tags– but if we moved to CSS variables for themes, it would enable us to use external CSS (with support for e.g. :hover states) in cases where it made more sense to.

I'm going to close this issue as a duplicate of #1090 since they're so closely related. Please feel free to add more thoughts to that issue if you'd like to contribute. 😄

@bvaughn bvaughn closed this as completed Sep 22, 2018
@infinnie
Copy link
Author

Improved UI together with

  • Pseudo classes
  • Pseudo elements
  • Key frame animations

Could be a tangible benefit for end users.

@bvaughn
Copy link
Contributor

bvaughn commented Sep 22, 2018

I don't see how pseudo classes or elements improves UI for end-users. They wouldn't even notice.

As for animations, DevTools doesn't currently animate anything except the profiler flame chart, and that's already built with CSS transitions. 😄

@infinnie
Copy link
Author

infinnie commented Sep 24, 2018

Say I want a different color for placeholders which behaves consistently in all browsers. What’s more, with CSS extracted to style tags more types of animation could be empowered.

@bvaughn
Copy link
Contributor

bvaughn commented Sep 24, 2018

Say I want a different color for placeholders which behaves consistently in all browsers.

I don't understand. This could be implemented with inline styles just as easily as with external styles.

What’s more, with CSS extracted to style tags more types of animation could be empowered.

True, but we don't have any use cases for new animations currently– so I don't think we should invest a lot of effort into that refactor.

@infinnie
Copy link
Author

infinnie commented Sep 26, 2018

Injected styles but not inline ones: inline styles do not support pseudo elements.

@bvaughn
Copy link
Contributor

bvaughn commented Sep 26, 2018

I did not realize you meant the :placeholder selector when you said "placeholders".

Regardless, you're still describing use cases that don't currently exist– so I don't think it's justified to spend any significant amount of effort to support them. 😄

@infinnie
Copy link
Author

infinnie commented Sep 26, 2018

The styles of pseudo elements behave inconsistently across browsers.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants