Skip to content
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

fix(dataZoom): fix restore zoom button to not skip undoing any zooming event #19119

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

blzsaa
Copy link

@blzsaa blzsaa commented Sep 14, 2023

fix(dataZoom): fix restore zoom button to not skip undoing any zooming event

close: #19118

Brief Information

This pull request is in the type of:

  • bug fixing
  • new feature
  • others

What does this PR do?

Pull request ensures that restore zoom button on dataZoom "Ctrl-Z-s" or undoes all zooming event including mouse scroll and slide.

Fixed issues

#19118

Details

Before: What was the problem?

Previously the undo function only applied to the click and hold zoom and not any other kind of zooming events.
It meant after mixing zooming events (e.g click and hold zoom + scroll + slide) undo functionality did skipped all zoom changes that was not made by click and hold zoom.

After: How does it behave after the fixing?

From now on the undo function will apply to any kind of zooming events.
It means that after mixing zooming events (e.g click and hold zoom + scroll + slide) undo functionality will undo them one by one until the it zooms out to the original range.

Document Info

One of the following should be checked.

  • This PR doesn't relate to document changes
  • 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

@echarts-bot
Copy link

echarts-bot bot commented Sep 14, 2023

Thanks for your contribution!
The community will review it ASAP. In the meanwhile, please checkout the coding standard and Wiki about How to make a pull request.

Copy link
Contributor

@Ovilia Ovilia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your contribution. Please follow the wiki to add some test cases. Thanks!

@blzsaa
Copy link
Author

blzsaa commented Sep 15, 2023

Hi, I added a test case: Zoom reset should undo all zooming event one by one

@github-actions
Copy link
Contributor

The changes brought by this PR can be previewed at: https://echarts.apache.org/examples/editor?version=PR-19119@ec410ea

Copy link
Contributor

@Ovilia Ovilia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your contribution! Please fix the lint problems.

I noticed that the restore behavior is not as expected after fixing. See this demo.

Currently, if you drag the selection slowly with the dataZoom slider, and then hit restore button in the toolbox, it will restore multiple times until reset. But it's expected to be reset directly once hit the restore button. Make sure the snapshot is taken only when the interaction is over (when mouse click ends).

@blzsaa
Copy link
Author

blzsaa commented Sep 19, 2023

  • The lint problem is fixed.
  • Moved the snapshot taking under _onDragEnd which will ensure that it is only called when the movement of the data slider is over.
  • As zooming by scroll has different dataZoomModel.id than zooming by click and hold history.pop was modified to be able to chain different kind of zoom events after each other.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug] restore zoom button works inconsistently
2 participants