Change Series Styling and Themes #15
Merged
Conversation
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 change removes the ability to configure the styling at the series level. As seen in this change and discussed in issue #14, these style options were not very broadly recognized. Instead the goal is to make theme customization easier. As a potential replacement to this configuration the Themes can now accept series and background color changes. If we do decide that we want to bring back per-series styling, I think we should make those top level fields so that we only introduce fiels which are broadly recognized.
Closed
This change removes the `FontSze`, `FontColor`, and `Font` from the `chartdraw.Style` struct and instead moves them to a `chartdraw.FontStyle`. This is the first change since forking `wcharczuk/go-chart` that will make a transition require additional changes. However I believe this is an important change. Using the broad `Style` struct makes it non-obvious what values are actually being used. In addition these font configurations are a common need, making an obvious choice for needing to bundle together. By removing the `Style` from `charts`, we instead are elevating these fields to be part of the public configuration. `Style` is now only used for rendering, and not part of the public API (unless using the `wcharczuk/go-chart` fork directly).
This change brings the `FontStyle` introduced in the prior commits into top level configuration. Although this change introduces significant changes into how charts are configured, being able to reuse font references will make unique font configurations easier.
407fe74 to
0e421a2
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This change removes the ability to configure the styling at the series level.
As seen in this change and discussed in issue #14, these style options were not very broadly recognized.
Instead the goal is to make theme customization easier. As a potential replacement to this configuration the Themes can now accept series and background color changes.
If we do decide that we want to bring back per-series styling, I think we should make those top level fields so that we only introduce fields which are broadly recognized.
As part of evaluating how the
Stylestruct was used, it was decided to break out the Font portions (which are used more broadly) into their ownFontStylestruct. This is a significant API change, both for users of the top level configuration, or users fromwcharczuk/go-chart. But one which should be easy to transition to, and will be better for the long term.In order to accommodate this change, at any point that
FontSize,FontColor, orFontwas being configured, this now must be encapsulated into aFontStylestruct. By having these values in a single struct you can now use several font configurations for different parts of the chart more easily.In the end this PR fixes #14 by ensuring that configuration fields in
chartsare always interpreted when rendered, allowing the user to be confident that if it's available to be set, it should have a visual impact.