-
Notifications
You must be signed in to change notification settings - Fork 178
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
Ability to customize vega-lite charts via configuration #726
Open
ajainarayanan
wants to merge
13
commits into
vega:master
Choose a base branch
from
ajainarayanan:feature/customizable-vega-charts
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
13 commits
Select commit
Hold shift + click to select a range
ac465db
Modifies voyager lib to accept vegaliteConfig as part of VoyagerConfig
ajainarayanan f17ecd2
Fixes reset reducer to not reset state values whose index is false
ajainarayanan 7522924
Modifies existing tests to use createVoyager api
ajainarayanan 570eb63
Adds new tests for newly added createVoyager API
ajainarayanan 93764db
Fixes Plot component to accept config parameter + modifies bookmark, …
ajainarayanan 3ceeef0
Removes old API + removes demo defaults
ajainarayanan 4a05ca9
Fixes unit test based on changes to lib-voyager
ajainarayanan f6cb91d
Adds additional unit test for makeResetReducer change
ajainarayanan e6cf103
Updates README
ajainarayanan 9fdaca1
Adds unit test for reset reducer
ajainarayanan cf2fede
Modifies reset reducer unit test to be independent
ajainarayanan 65b7bce
Fixes default data in lib-voyager + grammar fixes to unit tests
ajainarayanan da97efb
Fixes config parameter to be optional for createVoyager function
ajainarayanan File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
the general config should be overridden by a spec specific-config.
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.
Wouldn't this override the config users pass to modify vega-lite chart's configuration? My understanding was users pass
vegaliteConfig
to override spec configurations for respective charts (for instance modify the color or axis tick format etc.,)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.
Yes, it will override since we think about the shared config as a theme. So suppose you have multiple Vega-Lite charts and you have a common config that you pass in. Each spec can have config that override the common theme (rather than the other way round). :)
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.
If that is the case would the common theme ever be applied? Say in the case of bar charts we have defined the fill color to be a blue and if users need to override that would that be possible? I am not sure if I am missing something.
Can you provide an example where a user modifies the fill color of bar charts in voyager with custom config? My assumption was the config user provides are custom config and that would override any default spec config that comes as part of voyager for vega-lite charts.
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.
Yes, your common theme may have 10 properties and the spec specific theme may have 1 that override the common theme so 9 properties would still be applied.
Right now there is no such case. But this is how Vega-Lite treat "common" theme too. So should CompassQL output some config in the future.
That said, thinking more about it I think I agree with you that if users explicitly specify the config, it should override what's automatically generated by CompassQL.