You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@echarts.apache.org by "Skuperday (via GitHub)" <gi...@apache.org> on 2023/01/22 19:06:23 UTC
[GitHub] [echarts] Skuperday opened a new pull request, #18202: changed weakFilter logic, so line charts will not disappear, while zoom
Skuperday opened a new pull request, #18202:
URL: https://github.com/apache/echarts/pull/18202
<!-- Please fill in the following information to help us review your PR more efficiently. -->
## Brief Information
This pull request is in the type of:
- [x] bug fixing
- [ ] new feature
- [ ] others
### What does this PR do?
<!-- USE ONE SENTENCE TO DESCRIBE WHAT THIS PR DOES. -->
small change in weakFilter logic
it fixes some troubles with dataZoom in line charts, when line chart remain only 1-2 dots.
this allows us to use weakFilter to weed out the bar chart while we can also use a line chart
### Fixed issues
<!--
- #17858
- #12720
- #10646
- #17521
-->
## Details
### Before: What was the problem?
if we use filterMode: none with bar chart we get problem with exceed
but if we will use filterMode: weakFilter (as an example) our line charts on the same area with bar charts will disappear while zoom, if there is only 1-2 points (because there no way to make a line through 1 point)
with this simple change, line charts will be completed based on the values of the elements already existing on the chart
<!-- DESCRIBE THE BUG OR REQUIREMENT HERE. -->
<!-- ADD SCREENSHOT HERE IF APPLICABLE. -->
### After: How does it behave after the fixing?
<!-- THE RESULT AFTER FIXING AND A SIMPLE EXPLANATION ABOUT HOW IT IS FIXED. -->
<!-- ADD SCREENSHOT HERE IF APPLICABLE. -->
## Document Info
One of the following should be checked.
- [ ] This PR doesn't relate to document changes
- [x] The document should be updated later
- [ ] The document changes have been made in apache/echarts-doc#xxx
## Misc
### ZRender Changes
- [ ] This PR depends on ZRender changes (ecomfe/zrender#xxx).
### Related test cases or examples to use the new APIs
N.A.
## Others
### Merging options
- [ ] Please squash the commits into a single one when merging.
### Other information
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org
Re: [PR] changed weakFilter logic, so line charts will not disappear, while zoom [echarts]
Posted by "nick-true-dev (via GitHub)" <gi...@apache.org>.
nick-true-dev commented on PR #18202:
URL: https://github.com/apache/echarts/pull/18202#issuecomment-2300097002
FYI, I just tried @Skuperday's fix but got a Typescript error. So I had to make the following changes to the fix to work with the current build.
Anyway, thank you @Skuperday and @Ovilia for working on this issue!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org
Re: [PR] changed weakFilter logic, so line charts will not disappear, while zoom [echarts]
Posted by "Skuperday (via GitHub)" <gi...@apache.org>.
Skuperday closed pull request #18202: changed weakFilter logic, so line charts will not disappear, while zoom
URL: https://github.com/apache/echarts/pull/18202
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org
Re: [PR] changed weakFilter logic, so line charts will not disappear, while zoom [echarts]
Posted by "Skuperday (via GitHub)" <gi...@apache.org>.
Skuperday commented on PR #18202:
URL: https://github.com/apache/echarts/pull/18202#issuecomment-2368835023
> > Thanks alot to mr. @nick-true-dev, i update the PR and add following changes to my code, so this work fine and i'm ready to merge! please @Ovilia review my changes and make lot of people happy with resolving bugs:)
> > or maybe can some one else review this changes? @plainheart Can you help me?
>
> > Thanks alot to mr. @nick-true-dev, i update the PR and add following changes to my code, so this work fine and i'm ready to merge! please @Ovilia review my changes and make lot of people happy with resolving bugs:)
> > or maybe can some one else review this changes? @plainheart Can you help me?
>
> Any update on the review and merge :) ?
Currently waiting for review from @Ovilia, nothing to change
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org
Re: [PR] changed weakFilter logic, so line charts will not disappear, while zoom [echarts]
Posted by "Skuperday (via GitHub)" <gi...@apache.org>.
Skuperday commented on code in PR #18202:
URL: https://github.com/apache/echarts/pull/18202#discussion_r1747980468
##########
src/component/dataZoom/AxisProxy.ts:
##########
Review Comment:
'SupportedAxis' not in my changes but i can delete it if there no way to use it
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org
Re: [PR] changed weakFilter logic, so line charts will not disappear, while zoom [echarts]
Posted by "plainheart (via GitHub)" <gi...@apache.org>.
plainheart commented on PR #18202:
URL: https://github.com/apache/echarts/pull/18202#issuecomment-2105648169
@Skuperday You can dismiss that information.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org
Re: [PR] changed weakFilter logic, so line charts will not disappear, while zoom [echarts]
Posted by "Skuperday (via GitHub)" <gi...@apache.org>.
Skuperday commented on code in PR #18202:
URL: https://github.com/apache/echarts/pull/18202#discussion_r1796539203
##########
test/axis-weak-filter.html:
##########
Review Comment:
just test case that demonstrate diff in logic under weakFilter and without it
##########
src/component/dataZoom/AxisProxy.ts:
##########
Review Comment:
add the new version of implement, that don't generate errors on eslinter
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org
Re: [PR] changed weakFilter logic, so line charts will not disappear, while zoom [echarts]
Posted by "Skuperday (via GitHub)" <gi...@apache.org>.
Skuperday commented on PR #18202:
URL: https://github.com/apache/echarts/pull/18202#issuecomment-2105626082
> The changes brought by this PR can be previewed at: https://echarts.apache.org/examples/editor?version=PR-18202@635bfd9
https://echarts.apache.org/examples/en/editor.html?version=PR-18202%40635bfd9&code=MYewdgzgLgBAJgQygmBeGBtAUDTByARjwBoYCBdYnfAJhLMuozwGZ6KrdmAWdxrvAFY-nfADYRTPAHZJAgBxz8ATnYA6af3wEADOrFbmBIqQJrBhwnVNqWlgmxt3RR3jZrks5ANxYsIAAcoAEtwNBgAb2oADwBBaOCIAC5IgF9RAE94xJSI9OoIAFMAJ2DC5MjqXCgMgMKUvAAbYLBCkir4JAQUxGRqfNxehAAtEBAAWxTsXFwomZmAM2DGqBKAWRA4epg8AHdChABrADFl1eL2-dw4hIgASTAt6JSdUXmauoaIZq2Lt5mABaFYIAcwBUBSLFeHVwEABIF2ABEugBlAEIOAIlJQYoAV0K_1wACMQFAoBMUjQdB1UtRPKlvEA
Look at filter and then try to zoom into. My changes does line not disapear at zoom in. When actual master makes it disapear.
Please check the screenshots above to make sure it works correctly
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org
Re: [PR] changed weakFilter logic, so line charts will not disappear, while zoom [echarts]
Posted by "Skuperday (via GitHub)" <gi...@apache.org>.
Skuperday commented on PR #18202:
URL: https://github.com/apache/echarts/pull/18202#issuecomment-1925811238
@Ovilia Please check my changes. I added test cases and did not touch the files in dist.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org
Re: [PR] changed weakFilter logic, so line charts will not disappear, while zoom [echarts]
Posted by "elnipa (via GitHub)" <gi...@apache.org>.
elnipa commented on PR #18202:
URL: https://github.com/apache/echarts/pull/18202#issuecomment-1878666603
Any update on this PR? This would be extremely helpful as we are currently forced to use `none` filter, which is introducing bugs: https://github.com/apache/echarts/issues/18688
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org
Re: [PR] changed weakFilter logic, so line charts will not disappear, while zoom [echarts]
Posted by "Skuperday (via GitHub)" <gi...@apache.org>.
Skuperday commented on PR #18202:
URL: https://github.com/apache/echarts/pull/18202#issuecomment-2335092314
Thanks alot to mr. @nick-true-dev, i update the PR and add following changes to my code, so this work fine and i'm ready to merge!
please @Ovilia review my changes it's makes lot of people happy with resolving bugs:)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org
Re: [PR] changed weakFilter logic, so line charts will not disappear, while zoom [echarts]
Posted by "100pah (via GitHub)" <gi...@apache.org>.
100pah closed pull request #18202: changed weakFilter logic, so line charts will not disappear, while zoom
URL: https://github.com/apache/echarts/pull/18202
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org
[GitHub] [echarts] Skuperday commented on pull request #18202: changed weakFilter logic, so line charts will not disappear, while zoom
Posted by "Skuperday (via GitHub)" <gi...@apache.org>.
Skuperday commented on PR #18202:
URL: https://github.com/apache/echarts/pull/18202#issuecomment-1433555487
@Ovilia i can't find same logic in the src folder. I've tried to looking for "weakFilter" in project, but there only dist files.
How do i find where to make changes?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org
Re: [PR] changed weakFilter logic, so line charts will not disappear, while zoom [echarts]
Posted by "Skuperday (via GitHub)" <gi...@apache.org>.
Skuperday commented on PR #18202:
URL: https://github.com/apache/echarts/pull/18202#issuecomment-2105627105
@Ovilia
I don't know why some checks not successful, first one was succes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org
Re: [PR] changed weakFilter logic, so line charts will not disappear, while zoom [echarts]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #18202:
URL: https://github.com/apache/echarts/pull/18202#issuecomment-2105413333
<!-- ECHARTS_PR_PREVIEW -->
The changes brought by this PR can be previewed at: https://echarts.apache.org/examples/editor?version=PR-18202@635bfd9
<!-- ECHARTS_PR_PREVIEW -->
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org
Re: [PR] changed weakFilter logic, so line charts will not disappear, while zoom [echarts]
Posted by "100pah (via GitHub)" <gi...@apache.org>.
100pah commented on PR #18202:
URL: https://github.com/apache/echarts/pull/18202#issuecomment-4449545250
Many thanks to this investigating and fix.
The new v6.1.0 fixes this issue in a generic way instead of special handling.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org
Re: [PR] changed weakFilter logic, so line charts will not disappear, while zoom [echarts]
Posted by "Skuperday (via GitHub)" <gi...@apache.org>.
Skuperday commented on PR #18202:
URL: https://github.com/apache/echarts/pull/18202#issuecomment-2144885324
@Ovilia Can you review my changes?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org
Re: [PR] changed weakFilter logic, so line charts will not disappear, while zoom [echarts]
Posted by "therajat08 (via GitHub)" <gi...@apache.org>.
therajat08 commented on PR #18202:
URL: https://github.com/apache/echarts/pull/18202#issuecomment-2368065622
> Thanks alot to mr. @nick-true-dev, i update the PR and add following changes to my code, so this work fine and i'm ready to merge! please @Ovilia review my changes and make lot of people happy with resolving bugs:)
>
> or maybe can some one else review this changes? @plainheart Can you help me?
> Thanks alot to mr. @nick-true-dev, i update the PR and add following changes to my code, so this work fine and i'm ready to merge! please @Ovilia review my changes and make lot of people happy with resolving bugs:)
>
> or maybe can some one else review this changes? @plainheart Can you help me?
Any update on the review and merge :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org
[GitHub] [echarts] Ovilia commented on pull request #18202: changed weakFilter logic, so line charts will not disappear, while zoom
Posted by "Ovilia (via GitHub)" <gi...@apache.org>.
Ovilia commented on PR #18202:
URL: https://github.com/apache/echarts/pull/18202#issuecomment-1437768055
The logic is at https://github.com/apache/echarts/blob/master/src/component/dataZoom/AxisProxy.ts#L318
Here's how to debug: https://github.com/apache/echarts/wiki/How-to-setup-the-dev-environment#3-run-and-debug
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org
[GitHub] [echarts] Skuperday commented on pull request #18202: changed weakFilter logic, so line charts will not disappear, while zoom
Posted by "Skuperday (via GitHub)" <gi...@apache.org>.
Skuperday commented on PR #18202:
URL: https://github.com/apache/echarts/pull/18202#issuecomment-1559725480
Hello there!
As a test case, i'm does same changes in our version of library in our product. And my solution works well at this time.
Look how it going now vs how it being in past (without my changes)
before:
before but while zoom (look how disappear horizontal lines):
now:
and now while zoom:
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org
[GitHub] [echarts] echarts-bot[bot] commented on pull request #18202: changed weakFilter logic, so line charts will not disappear, while zoom
Posted by echarts-bot.
echarts-bot[bot] commented on PR #18202:
URL: https://github.com/apache/echarts/pull/18202#issuecomment-1399576126
Thanks for your contribution!
The community will review it ASAP. In the meanwhile, please checkout [the coding standard](https://echarts.apache.org/en/coding-standard.html) and Wiki about [How to make a pull request](https://github.com/apache/echarts/wiki/How-to-make-a-pull-request).
Document changes are required in this PR. Please also make a PR to [apache/echarts-doc](https://github.com/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 `PR: awaiting doc` label.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org