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

feat(settings): expose global charts context for settings #1424

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

nickofthyme
Copy link
Collaborator

@nickofthyme nickofthyme commented Oct 13, 2021

Summary

adds ElasticChartsProvider to pass EchContext value to set globally shared values across all chart instances. This remediates the need to provide a theme and baseTheme to all chart instances, the theme will just be merged with any provided user theme, when the chart is wrapped in the provider.

export interface EchContext {
  settings?: SettingsProps;
}

used as...

<ElasticChartsProvider value={{ settings: mySettings }}>
  <Chart>
  <Settings /> {/* must provide spec to inherit from provider context */}
    {/* ...component specs */}
  </Chart>
</ElasticChartsProvider>

Details

Leaving the api for EchContext open for future props beyond just settings.

Issues

This completes a long ignored usage for shared global settings
fix #718

Depends on #1421

Checklist

  • The proper chart type label has been added (e.g. :xy, :partition)
  • The proper feature labels have been added (e.g. :interactions, :axis)
  • The :theme label has been added and the @elastic/eui-design team has been pinged when there are Theme API changes
  • All related issues have been linked (i.e. closes #123, fixes #123)
  • New public API exports have been added to packages/charts/src/index.ts
  • Unit tests have been added or updated to match the most common scenarios
  • The proper documentation and/or storybook story has been added or updated
  • The code has been checked for cross-browser compatibility (Chrome, Firefox, Safari, Edge)
  • Visual changes have been tested with all available themes including dark, light, eui-dark & eui-light

@nickofthyme nickofthyme added enhancement New feature or request kibana cross issue Has a Kibana issue counterpart :all Applies to all chart types :theme labels Oct 13, 2021
@nickofthyme nickofthyme requested a review from markov00 October 13, 2021 23:10
@nickofthyme
Copy link
Collaborator Author

We have discussed moving the eui theme into charts and providing a single theme. This could be useful to easily access the isDarkMode variable inside kibana.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:all Applies to all chart types enhancement New feature or request kibana cross issue Has a Kibana issue counterpart :theme
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Better shared theming
1 participant