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

Better shared theming #718

Open
1 task done
nickofthyme opened this issue Jun 23, 2020 · 1 comment · May be fixed by #1424
Open
1 task done

Better shared theming #718

nickofthyme opened this issue Jun 23, 2020 · 1 comment · May be fixed by #1424
Labels
discuss To be discussed enhancement New feature or request :styling Styling related issue

Comments

@nickofthyme
Copy link
Collaborator

nickofthyme commented Jun 23, 2020

Is your feature request related to a problem? Please describe.
Currently, in kibana we use a Charts plugin to share a theme across all Chart instances. This is done by using either an observable or a react hook to get the theme and baseTheme then pass them to the Settings Chart component.

This is very repetitive and often overlooked, to where only two of the 20 or so usages are using this theme service.

Describe the solution you'd like
I'd like a way to share a component across all usages that implements the theming logic, then people can simply import this new ThemedChart component and override any theme values they desire.

Some kind of HOC that passes a baseTheme and theme to the Settings component. The issue is that you cannot simply create a HOC with props on a nested Child component. This would require changes to allow this functionality.

Maybe an easier but less ideal approach could be to provide a ThemedSettings HO component in kibana that passes around the correct theming. This would not require any changes to EC. Something like...

const MyComponent = () => (
  <Chart>
    <ThemedSettings theme={EuiThemeOverrides} {...otherSettingsProps} />
    {/* more goodness */}
  </Chart>
)

Checklist

  • this request is checked against already exist requests
@nickofthyme nickofthyme added enhancement New feature or request discuss To be discussed :styling Styling related issue labels Jun 23, 2020
@markov00
Copy link
Member

markov00 commented Jul 9, 2020

Yep definitely, I think we already discussed that with Caroline years ago, the ideas was to have a ThemedSettings exactly as you described. Here is an example: https://codesandbox.io/s/focused-satoshi-wrvi7?file=/src/App.tsx

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discuss To be discussed enhancement New feature or request :styling Styling related issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants