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

v0.4.0 API Simplifications and Uniformity Changes #27

Merged
merged 16 commits into from
Jan 12, 2025

Conversation

jentfoo
Copy link
Member

@jentfoo jentfoo commented Jan 1, 2025

v0.4.0 will be focused on minimizing and simplifying the public API so that our module is easier to learn. This will include removing public structs and functions not expected to be utilized by the user, as well as making sure the public API presented is consistent.

Although these changes look significant, what is expected to be user impacting API changes should be demonstrated by the changes in examples. We expect the user impacts to be very small, with most of the API removal to be functionality not expected to be used externally.

If anything removed or hidden in this PR / release is useful to you, please open an issue referencing this PR. The API cleanup is expected only to make it easier to learn and adopt our library, not reduce functionality.

Config Changes:

  • LegendOptions.Vertical introduced in Offset, Orientation and other config ergonomict tweaks #20 has been changed from a bool to *bool. This makes the field more consistent to other configuration, and provides more future flexibility for dynamic defaults.
  • LineChartOption.StrokeWidth has been renamed to LineStrokeWidth to be more consistent with other configurations.
  • Removed OutputFormatOptionFunc, instead use SVGOutputOptionFunc() or PNGOutputOptionFunc()
  • Dimensions are now always set together, WidthOptionFunc and HeightOptionFunc are now set with DimensionsOptionFunc. Similar SetDefaultWidth and SetDefaultHeight were replaced with SetDefaultChartDimensions.

Series Changes:

  • SeriesList are now constructed in a more consistent way with a function specific to the type. This removes from the public API NewSeriesListDataFromValues and NewSeriesFromValues. Instead of optionally passing in a chart type it's now expected to call a function specific to the chart type: NewSeriesListX (with X being the chart type). Though manual SeriesList construction is less needed, in general chart Options structs are easier to construct by using NewXChartOptionWithData (for example: TestNewLineChartOptionWithData).
  • SeriesData struct has been removed, instead we just use a float64 slice in Series to reference the values directly.

Painter Changes:

  • Public NewXPainter functions are made private, except for NewPainter, these other cases are expected to be used internally.
  • When constructing a Painter through NewPainter there is now no error return
  • Renderer type has been made private, and New functions to create these structs has been removed. For rendering charts using the Painter API, these functions are now invoked on a Painter instance, for example:
    • NewBarChart -> Painter.BarChart
    • NewHorizontalBarChart -> Painter.HorizontalBarChart
    • NewFunnelChart -> Painter.FunnelChart
    • NewLineChart -> Painter.LineChart
    • NewPieChart -> Painter.PieChart
    • NewRadarChart -> Painter.RadarChart
    • NewTableChart -> Painter.TableChart
  • Functions on Painter have been reduced to only post-rendered chart modifications a user may want to do. For example, drawing arrows, adding text, additional lines, circles or boxes, etc. As part of this ArrowTop and ArrowBottom were renamed to ArrowUp and ArrowDown. Additionally functions on the Painter don't return a Painter instance unless the action only applies to the returned instance.
  • The style in Painter (ie fill color, stroke color, stroke width, etc) is no longer stateful, instead these style attributes are accepted as function arguments. In general functions have changed on the Painter to make it more obvious what is useful for external users.

Other Removals and Changes:

  • SVG rendering improvements result in smaller SVG files by omitting useless drawing directives.
  • LabelFormatter has been removed (not to be confused with ValueFormatter, which remains)
  • BoxOptionFunc was removed, we don't currently have a clear example of where this is useful.
  • Public structs AxisOption (and similar the ToAxisOption function), GridPainterOption, SeriesLabelPainter, SeriesLabelPainterParams, have been made private or removed. These are expected to only be used internally.
  • Grid painter functionality was removed (not needed internally, and not clear how it would be useful to the end user)

@jentfoo jentfoo added the enhancement New feature or request label Jan 1, 2025
@jentfoo jentfoo self-assigned this Jan 1, 2025
@jentfoo jentfoo changed the title v0.4.0 Config API Changes v0.4.0 Config API Simplifications and Uniformity Changes Jan 1, 2025
painter.go Show resolved Hide resolved
painter.go Outdated Show resolved Hide resolved
painter.go Show resolved Hide resolved
painter.go Show resolved Hide resolved
painter.go Show resolved Hide resolved
painter.go Show resolved Hide resolved
pie_chart.go Show resolved Hide resolved
radar_chart.go Show resolved Hide resolved
@jentfoo jentfoo force-pushed the jent/v0.4.0-config_api branch 5 times, most recently from 8c80514 to 477c66a Compare January 3, 2025 19:58
painter.go Outdated Show resolved Hide resolved
xaxis.go Show resolved Hide resolved
yaxis.go Outdated Show resolved Hide resolved
@jentfoo jentfoo force-pushed the jent/v0.4.0-config_api branch from 8c4b45c to 34619ec Compare January 10, 2025 04:27
funnel_chart.go Outdated Show resolved Hide resolved
@jentfoo jentfoo force-pushed the jent/v0.4.0-config_api branch 3 times, most recently from b380f4f to 0b28abc Compare January 10, 2025 06:13
@jentfoo jentfoo changed the title v0.4.0 Config API Simplifications and Uniformity Changes v0.4.0 API Simplifications and Uniformity Changes Jan 10, 2025
@jentfoo jentfoo force-pushed the jent/v0.4.0-config_api branch from 95193fb to b0a39e6 Compare January 11, 2025 20:40
v0.4.0 will be focused on minimizing and simplifying the public API so that our module is easier to learn.  This will include removing public structs and functions not expected to be utilized by the user, as well as making sure the public API presented is consistent.

Changes included so far:
  * `LegendOptions.Vertical` introduced in #20 has been changed from a `bool` to `*bool`.  This makes the field more consistent to other configuration, and provides more future flexibility for dynamic defaults.
  * Public `NewXPainter` functions are made private, these are only expected to be used internally.
  * Public structs `AxisOption`, `GridPainterOption`, `SeriesLabelPainter`, `SeriesLabelPainterParams`, have been made private or removed.  These are expected to only be used internally.
  * `LabelFormatter` has been removed (not to be confused with `ValueFormatter`, which remains)
  * Additional functions expected to be internal only: `NewRange`, `NewLeftYAxis`, `NewRightYAxis`, `NewBottomXAxis`, `NewEChartsSeriesDataValue` (struct is still public if needed)
  * `PainterOption` function has been renamed to `PainterOptionFunc`
  * Removed `OutputFormatOptionFunc`, instead use `SVGOutputOptionFunc()` or `PNGOutputOptionFunc()`
  * Dimensions are now always set together, `WidthOptionFunc` and `HeightOptionFunc` are now set with `DimensionsOptionFunc`.  Similar `SetDefaultWidth` and `SetDefaultHeight` were replaced with `SetDefaultChartDimensions`.
  * `BoxOptionFunc` was removed, we don't currently have a clear example of where this is useful.
Functions on `Painter` have been reduced to only post-rendered chart modifications a user may want to do.  For example, drawing arrows, adding text, additional lines, circles or boxes, etc.

As part of this `ArrowTop` and `ArrowBottom` were renamed to `ArrowUp` and `ArrowDown`.
This was previously identified as internal, but on closer inspection I think it should be removed.  This is not used by the table creation, so I would like a cleaer understanding of how it's used (and add an example of it) before continuing to support it.
This struct only contained a single field `Value` which was a float64.  To reduce the API surface area this struct is being removed and instead the `Series` struct will contain a slice of `float64` directly for the `Data`.
The prior TODO note to make the fields lower case is not happening in this version.  Instead we want to help elevate these statistics in case they are useful to the user.
This removes from the public API NewSeriesListDataFromValues and NewSeriesFromValues.  Instead of optionally passing in a chart type it's now expected to call a function specific to the chart type: NewSeriesListLine, NewSeriesListBar, NewSeriesListHorizontalBar, NewSeriesListPie, NewSeriesListRadar, or NewSeriesListFunnel.

NewPieSeriesList and NewFunnelSeriesList previously existed, so this API makes the construction more uniform for all chart types (note that these have been renamed to have a `NewSeriesList` prefix to the function name).

While examining the Options struct passed in (which has now been adopted for all series types), it was noted that Pie rendering almost always passed in a SeriesLabel to enable rendering the labels.

The examples demonstrate these API changes well, hopefully this API is more concise and easier to learn.
These fields were only used in the Funnel charts, and the usage not currently tested.  Removing this functionality until it's use case is clearly understood and tested.
This removes the error return on `chartdraw.RendererProvider`.  Currently the only error return was for a PNG case that was due to a type checking that would have been from a bug.  Instead the function signatures were changed so the type check was no longer necessary, making the error result no longer needed.

The above change was able to cascade until `charts.NewPainter` no longer needed to return an error result either.
I don't like that this API implied an immutability in the Painter that did not exist.

Code can be easily restructured to call into the Painter reference sequentually (Minus the `Child` action, which does in fact return a new instance)
This commit removes the need for external users to be aware of Renders (`Renderer` type made private in a prior commit).  Instead if a user wants to render charts on a Painter they can invoke the relevant function on the painter:
* NewBarChart -> Painter.BarChart
* NewHorizontalBarChart -> Painter.HorizontalBarChart
* NewFunnelChart -> Painter.FunnelChart
* NewLineChart -> Painter.LineChart
* NewPieChart -> Painter.PieChart
* NewRadarChart -> Painter.RadarChart
* NewTableChart -> Painter.TableChart

In additional `TableOptionRender` was renamed to `TableOptionRenderDirect` and `TableRender` was renamed to `TableRenderValues`, both to provide clarity because these render functions are unique compared to others.
This name seems more obvious to match with the more generalized `ChartOptions`
The axisOption struct is internal, only used to minimize code duplication between the x and y axis.
@jentfoo jentfoo force-pushed the jent/v0.4.0-config_api branch 2 times, most recently from 1a53809 to cd51aaf Compare January 12, 2025 04:50
Changes put together because both required significant changes to the SVG validation in tests.  These changes were carefully examined to make sure there were no negative visual issues.

**Painter API changes**:
The Painter API can be confusing, with some functions depending on the state of the painter.
This changes the Painter API (internal and external) so that style attributes (ie FillColor, StrokeColor, StrokeWidth, etc) are not stateful. Instead the relevant style values are accepted as arguments to the draw action.

This creates an easier to use API.  It also addresses a complexity if multiple Child painters are used together with different drawing state.

Additionally the Painter configuration has changed.  A default theme and default font is still available to set in the Painter, however the `ValueFormatter` was removed.  Instead this config field only exists in charts options which make use of a set formatter.

Test SVG's resulted from two primary aspects of this change:
  * Measuring text not representing the actual text size due to the font style not set correctly in the Painter
  * A fill style attribute being specified when it has no effect (since style was set due to prior operations)

In general the exposed functions were general examined to ensure all exposed functions are both useful, and easy to use.  This resulted in a number of argument changes, which should be detailed in the go docs.

**vector_renderer (SVG) improvements**:

This change minimizes and simplifies the SVG output through the following:
  * If stroke width is zero, or the color is transparent only `stroke:none` will be set, instead of the prior `stroke-width:0;stroke:none` value
  * Colors will be specified by name if possible, otherwise an RGB or RGBA string will still be used
  * If the text or path is empty, nothing will be inserted into the svg.

These three changes reduce the SVG size.  Specifying the color names could potentially allow color shade changes using CSS.

As a bonus, additional colors are now specified in the `drawing` package.

----

SVG changes are always painful, these were careful evaluated to ensure this is an improvement in both the Painter functionality and SVG results.
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 jentfoo force-pushed the jent/v0.4.0-config_api branch from cd51aaf to 26851a3 Compare January 12, 2025 05:46
@jentfoo jentfoo merged commit 26851a3 into main Jan 12, 2025
8 checks passed
@jentfoo jentfoo deleted the jent/v0.4.0-config_api branch January 15, 2025 02:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant