-
Notifications
You must be signed in to change notification settings - Fork 19.7k
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
feat: bar race #12484
feat: bar race #12484
Conversation
Thanks for your contribution! |
src/coord/cartesian/Grid.ts
Outdated
@@ -447,6 +462,36 @@ class Grid implements CoordinateSystemMaster { | |||
); | |||
}); | |||
} | |||
|
|||
// function sortCategory( | |||
// data: List, |
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.
Remove the commented code if they are not used anymore
setCategorySortInfo(info: OrdinalSortInfo[]): void { | ||
this._categorySortInfo = info; | ||
} | ||
|
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.
I can't find the general logic of sorting the categories when realtimeSort
is not set. Did I miss it?
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.
It's not implemented yet. It will be fixed later.
@@ -35,6 +36,10 @@ interface CartesianAxisOption extends AxisBaseOption { | |||
position?: CartesianAxisPosition; | |||
// Offset is for multiple axis on the same position. | |||
offset?: number; | |||
sort?: boolean; | |||
realtimeSort?: boolean; | |||
sortSeriesIndex?: number; |
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.
Still not very show if it's better to put sort
realtimeSort
configuration on the series. Perhaps we can let developers do the decision.
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.
Leave this as it is for now.
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.
Commented my questions.
Congratulations! Your PR has been merged. Thanks for your contribution! 👍 |
@Ovilia Hi, thank you for your excellent work for bar race. Here I'd like to give some feedback for this new feature, please feel free to check and evaluate it. What I think it's expected is like this: That is, What do you think of these? EDITED: I saw it looks like working well in the screenshot you provided in PR description. |
Yes, it should change from previous values. I think there might be a bug with this. Not sure about it. I will check it later. |
Brief Information
This pull request is in the type of:
What does this PR do?
Enables bar race for ECharts 5.0.
Fixed issues
Details
Before: What was the problem?
Bar race is a popular way to show time series data, and visualize the change in trends over time. See examples at flourish and amcharts.
This PR brings this feature to ECharts including:
It still have some bugs when axis has
min
ormax
to restrict on the bars to be filtered, which should be fixed in further PRs.After: How is it fixed in this PR?
Usage
Are there any API changes?
New options:
Typical usage:
Related test cases or examples to use the new APIs
test/bar-race.html
Others
Merging options
Other information