Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

EuiText and EuiIcon colors should change when wrapped in EuiThemeProvider #6123

Closed
elizabetdev opened this issue Aug 10, 2022 · 2 comments · Fixed by #6155
Closed

EuiText and EuiIcon colors should change when wrapped in EuiThemeProvider #6123

elizabetdev opened this issue Aug 10, 2022 · 2 comments · Fixed by #6155
Labels

Comments

@elizabetdev
Copy link
Contributor

When the EuiText and EuiIcon are wrapped in a <EuiThemeProvider color="dark" /> their color should be white. Both components are already converted to Emotion so I'm expecting that they should adapt their styles to the EuiThemeProvider.

The components use color: inherit; and fill: currentColor;. So I'm not sure if this is really an issue or a lack of documentation. Somewhere in our docs maybe we should say that some components inherit the parent color. So when the color mode changes we also need to change the parent color so that components can inherit the right color... 🤷🏽‍♀️

But as a user, I would expect the color to change... And everything to work out of the box.

I created a CodesandBox that shows the issue :

Screenshot 2022-08-10 at 15 58 57

@cee-chen
Copy link
Contributor

Just curious - why do we use color: inherit? Why not simply have EuiText specify our base text color by default, since we allow consumers to override color via the color prop in any case?

@elizabetdev
Copy link
Contributor Author

The color: inherit was added here #770. I guess it doesn't make more sense to have this.

The EuiText can have the "color="default" by default. The color can always be overridden by passing a color to the color prop, so I don't see any problem here.

For EuiIcon I think we can havecolor="text" by default. Then we would need to make sure all the components that are using EuiIcon are passing the expected color to the component like in here:

Screenshot 2022-08-10 at 18 25 39

I'm going also to investigate in Kibana if having default and text colors by default would impact any usage. But they could always change the color to inherit.

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

Successfully merging a pull request may close this issue.

2 participants