-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Add Yield Curve and Options Chart Types, Up/Down Markers Plugin #1674
base: v5-candidate
Are you sure you want to change the base?
Conversation
size-limit report 📦
|
/** | ||
* Defines the supported series types for markers. | ||
*/ | ||
export type SupportedSeriesTypes = 'Line' | 'Area'; |
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.
Since this is exported and thus included in the generated d.ts
file lets give it a more explicit name
}); | ||
const priceChangeMarkers = new UpDownMarkersPrimitive(); | ||
series1.attachPrimitive(priceChangeMarkers); | ||
priceChangeMarkers.setData(curve1); |
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.
Not a review, just a question. Did you consider creating it using a custom series? From my point of view, managing the series via the series' primitive seems unintuitive. Since it’s a core plugin, we have access to all the source code, so we could potentially wrap the line and area series and add the up/down indicator logic on update. I haven’t thought much about it, so there might be other limitations with the custom series approach
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.
As with most things it is a trade-off. In many ways it makes sense to create a custom series but there are some benefits to using a Primitive. The drawing of markers fits better into a primitive (conceptually), and the main reason for using a primitive is that it can be used on multiple types of series (and custom series) so we are not limiting this plugin to only working with one or two series which are copies of existing series types.
Finally, the up down markers plugin can be used without needing to use the setData
/ update
passthrough methods if you use the manual setMarkers
method. So my thinking is that using as a Primitive is a more flexible and lower level implementation which can make it useful in more use-cases.
Type of PR: new features
PR checklist:
Overview of change:
Add Yield Curve and Options Chart Types, Up/Down Markers Plugin, and Documentation Updates
New Features:
Yield Curve Chart:
createYieldCurveChart
function.Options Chart:
createOptionsChart
function.Up/Down Markers Plugin:
UpDownMarkersPrimitive
plugin to display markers indicating price changes.Codebase Improvements / API additions:
ignoreWhitespaceIndices
option to the time scale configuration.updateSeriesData
function to support historical updates.Documentation: