-
Notifications
You must be signed in to change notification settings - Fork 3
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
refactor: Move dataSet from configurator state to chart state #1240
Conversation
...as we don't have a chart yet, it doesn't bring a lot of value; and with the recent change of storing dataSet in ChartConfig, we can't really access chart config when selecting a dataset.
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nothing looks too out of place. Nice to be able to remove that guards when !stata.dataSet
👍
There are a few places where you replace state.dataSet
with chartConfig.dataSet
but my understanding was that chartConfig
could either be an object or an array of objects 🤔 is this not the case or are all these cases when the configurator has been split up for each chart?
Discussed with @lloydrichards offline; basically we are always in the context of a chart config, which we retrieve from the configurator state using the |
This PR provides the technical foundation for the dashboards to allow charts based on different datasets within the same configurator state.
Essentially, it moves the
dataSet
property from the configurator state down to the charts themselves.How to test
vizualize-configurator-state:EpXqRKzyJxIy
key):/en/create/EpXqRKzyJxIy?dataSource=Prod
.