-
Notifications
You must be signed in to change notification settings - Fork 19.8k
Fix #18206 - [Feature] Enable indicators Radar charts to invert their axis #18376
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
Conversation
|
Thanks for your contribution! Document changes are required in this PR. Please also make a PR to apache/echarts-doc for document changes and update the issue id in the PR description. When the doc PR is merged, the maintainers will remove the |
| indicatorAxis.name = indicatorModel.get('name'); | ||
| // Inject model and axis | ||
| indicatorAxis.model = indicatorModel; | ||
| indicatorAxis.inverse = indicatorModel.get('inverse'); |
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.
This is probably not necessary to be set to indicatorAxis.inverse. You should call indicatorModel.get('inverse') whenever you need the value.
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.
Unfortunately, without this line the test graph does not render as expected. I think the Axes needs to know it is inversed as well as the data model which is why this line is there? I assumed this is how it is achieved on the other charts which support the inverse property as the base Axis class has the inverse property set.
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.
Thanks for the information. Yes, others may depend on this information.
Ovilia
left a comment
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've run visual tests about radar cases and all passed.
| indicatorAxis.name = indicatorModel.get('name'); | ||
| // Inject model and axis | ||
| indicatorAxis.model = indicatorModel; | ||
| indicatorAxis.inverse = indicatorModel.get('inverse'); |
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.
Thanks for the information. Yes, others may depend on this information.
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.
@jasonpaige Please check the failed test case in the CI. It's werid that the failed case is not about radar. Please let me know if you fail to figure it out.
|
Andy progress on this? |
|
Also happen to have a need for this currently, looks like the CI logs are not available anymore though. |
|
This PR has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. If you wish not to mark it as stale, please leave a comment in this PR. We are sorry for this but 2 years is a long time and the code base has been changed a lot. Thanks for your contribution anyway. |
|
This PR has been automatically closed because it has not had recent activity. Sorry for that and we are looking forward to your next contribution. |
Brief Information
This pull request is in the type of:
What does this PR do?
This PR adds the ability to 'inverse' any of the axis on a radar chart
Fixed issues
#18206
Details
Before: What was the problem?
Previously a radar chart could look like so:

However, you may which the cost to actually reflect negatively, the more expensive it is.After: How does it behave after the fixing?

Now this single axis is inverted, where the cheaper the cost, the better it appears on the radar chart.Document Info
One of the following should be checked.
Misc
Related test cases or examples to use the new APIs
A test case has been added here: test/radar-inverse.html
The API to inverse an indicator axis works by simply adding
inverse: truelike this: