Offset, Orientation and other config ergonomict tweaks #20
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 moves the `Top` and `Left` configuration options for a Legend or Title under the `OffsetStr` struct. In cases where only `Left` was being set with `PositionLeft` / `PositionCenter` / `PositionRight`, now `OffsetLeft` / `OffsetCenter` / `OffsetRight` can be used to set the `Offset` field. In addition sub-structs in configurations should consistently have `WithX` functions to allow easy modifications or a builder pattern.
Instead of specifying "vertical" as a string, the boolean can now be set directly.
In general we expect the configuration structs to be made directly by the user. These helper functions were not well documented and outside of how other configuration structs are initialized. For that reason we removed these three functions, and instead expect the struct to be initialized directly.
jentfoo
changed the title
jentfoo
changed the title
jentfoo
added a commit
that referenced
this pull request
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. 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)
jentfoo
added a commit
that referenced
this pull request
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. 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)
jentfoo
added a commit
that referenced
this pull request
Jan 2, 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. 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)
jentfoo
added a commit
that referenced
this pull request
Jan 2, 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. 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)
jentfoo
added a commit
that referenced
this pull request
Jan 3, 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. 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)
jentfoo
added a commit
that referenced
this pull request
Jan 3, 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. 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)
jentfoo
added a commit
that referenced
this pull request
Jan 3, 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. 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.
jentfoo
added a commit
that referenced
this pull request
Jan 10, 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. 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.
jentfoo
added a commit
that referenced
this pull request
Jan 10, 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. 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.
jentfoo
added a commit
that referenced
this pull request
Jan 12, 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. 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.
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 PR continues trying to tweak the chart configuration to be easier and hopefully more obvious. Changes included:
TopandLeftconfiguration options for a Legend or Title under theOffsetStrstruct. In cases where onlyLeftwas being set withPositionLeft/PositionCenter/PositionRight, nowOffsetLeft/OffsetCenter/OffsetRightcan be used to set theOffsetfield.OffsetStrorFontStyle) should consistently haveWithXfunctions to allow easy modifications or a builder pattern.Orientstring, which previously could only have valueshorizontalandverticalis now a boolean namedVertical, resulting inOrientationHorizontalandOrientationVerticalconstants being no longer needed and thus removed.NewLegendOptionNewXAxisOptionNewYAxisOptionswere removed. In general we expect the configuration structs to be made directly by the user. These helper functions were not well documented and outside of how other configuration structs are initialized. For that reason we removed these three functions, and instead expect the struct to be initialized directly.See
examplesfor examples on how to update your implementations. The changes here are similar to how most people are expected to update:https://github.com/go-analyze/charts/pull/20/files#diff-0a8fe8d29976febc4342e445372f0f0c7e7322acfaab81562ed1595f0f8d2f98R62-R67
Thoughts on the API:
I agonized over these API changes pretty heavily. I dislike having both an
OffsetStrandOffsetInt, however there are two cases needed.OffsetIntis used in cases where an absolute offset is provided, and a relative offset (for example50%) does not make sense.Additionally I dislike for the Legend how there are multiple configuration values which determine the position (
OffsetandPaddingboth). However I am unsure how to reduce these fields while keeping the configuration easy to understand.If you have thoughts on the above please put a comment on this PR or open an Issue. I want to continue to make chart configuration simpler and obvious while still being highly flexible!