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.

Sign up for GitHub

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

Jump to bottom

Using Painter API does not fill in configuration defaults correctly #30

Closed
jentfoo opened this issue Jan 3, 2025 · 0 comments
Closed

Using Painter API does not fill in configuration defaults correctly #30

jentfoo opened this issue Jan 3, 2025 · 0 comments
Labels
bug Something isn't working

Comments

@jentfoo
Copy link
Member

jentfoo commented Jan 3, 2025

In a recent commit we added examples using the Painter API. This is an API we want to continue to expand because the direct struct instantiation and use is often simpler. However using these examples we see that the defaults applied in ChartOptions.fillDefaults is not applied to these configuration structs.

In v0.4.0 we plan to continue to support both of these API's. We should see if we can find a way to make the default behaviors using both API's match. Possibly unifying the code flow further if possible.
@jentfoo jentfoo added the bug Something isn't working label Jan 3, 2025
jentfoo added a commit that referenced this issue Jan 12, 2025
@jentfoo
Add constructor functions for creating chart Option structs
958571e 
These constructor functions makes it easier to setup defaults, as well as call the specific function needed to build the SeriesList for the struct type.

This resolves #30
jentfoo added a commit that referenced this issue Jan 12, 2025
@jentfoo
Move series and legend association to defaultRender
1a53809 
This also is part of solving #30, this default operation should happen at this common point instead of fillDefaults().  In general fillDefaults() should match what we are doing in our constructor functions added in the prior commit.
jentfoo added a commit that referenced this issue Jan 12, 2025
@jentfoo
Introduce Options constructors for filling in defaults
cd51aaf 
This change solves bringing the default functionality more equal with both API's.
* Introduced Option struct constructors for each chart type - These constructor functions makes it easier to setup defaults, as well as call the specific function needed to build the SeriesList for the struct type.
* Moved chart series and legend setup to `defaultRender` - defaultRender is a common logic point we should use, with fillDefaults being very similar to the constructors introduced above.
* defaultRenderOption now has pointers for structs modified in rendering (another place for defaults filled in).  This internal API could likely be improved long term, but this is a small change to help ensure logic remains consistent.

Overall this change resolves #30
jentfoo added a commit that referenced this issue Jan 23, 2025
@jentfoo
Add BarMargin option to Bar and HorizontalBar charts
e48c5ed 
This option allows users to control the space between each bar within a series (and in effect the space between series).

In addition a bug was discovered where the Painter API may not correctly scale the chart when Markpoints are used. This bug is another permutation of #30 that was undiscovered, and fixed by moving this logic under `defaultRender`.
@jentfoo jentfoo mentioned this issue Jan 23, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@jentfoo