echarts icon indicating copy to clipboard operation
echarts copied to clipboard

feat: add the insideDataZoom 'gestureOnTouchPad' option that can Optimize the Experience on the MAC touchpad.

Open jianqi-jin opened this issue 2 years ago • 11 comments

Brief Information

This PR focuses on enhancing the experience when we just use the MAC touchpad to move or scale the dataZoom. fix: #17286 This pull request is in the type of:

  • [ ] bug fixing
  • [x] new feature
  • [ ] others

What does this PR do?

Fixed issues

Details

Before: What was the problem?

We can't use the touchpad to move the dataZoom. We just can scale it. And I wish that we can use the touchpad to move the dataZoom window. Such as lots of software, they can use the touchpad to scroll x direction and y direction both.

You can see the test file in the test/gesture.html see the result.

I added an option gestureOnTouchPad: Users can set gestureOnTouchPad to true, that means we adjust the MAC touchpad.


                  dataZoom: [
                    {
                      type: 'inside',
                      xAxisIndex: [0],
                      start: 1,
                      zoomOnMouseWheel: true,
                      gestureOnTouchPad: true, // this option
                      end: 35
                    },
                    {
                      type: 'inside',
                      yAxisIndex: [0],
                      zoomOnMouseWheel: true,
                      start: 29,
                      gestureOnTouchPad: true, // this option
                      end: 36
                    }
                  ],

https://user-images.githubusercontent.com/33452468/176181799-6cc6732b-4e08-4c95-8557-0feb98ba874d.mov

After: How does it behave after the fixing?

Document Info

One of the following should be checked.

  • [ ] This PR doesn't relate to document changes
  • [x] The document should be updated later
  • [x] 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

jianqi-jin avatar Jun 28 '22 12:06 jianqi-jin

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.

⚠️ MISSING DOCUMENT INFO: Please make sure one of the document options are checked in this PR's description. Search "Document Info" in the description of this PR. This should be done either by the author or the reviewers of the PR.

echarts-bot[bot] avatar Jun 28 '22 12:06 echarts-bot[bot]

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 PR: awaiting doc label.

echarts-bot[bot] avatar Jun 28 '22 12:06 echarts-bot[bot]

Please remove the changes of dist and i18n.

Okay, I removed them and fixed some lint error. Could you help me to review it again, and maybe find something inappropriate? @Ovilia

jianqi-jin avatar Jul 01 '22 06:07 jianqi-jin

Hi, Anything update here?

jianqi-jin avatar Jul 22 '22 03:07 jianqi-jin

Personally, I think B is better, cause touchpad is so different with mouse, and I think we should offer users to decide how to deal with wheel event.

jianqi-jin avatar Jul 28 '22 09:07 jianqi-jin

Any thing update here ~

jianqi-jin avatar Aug 19 '22 10:08 jianqi-jin

@100pah Please help review this PR.

Ovilia avatar Sep 06 '22 02:09 Ovilia

@100pah, Is there any thing bad? If so, I would like to fix it. Thanks. :)

jianqi-jin avatar Sep 08 '22 14:09 jianqi-jin

@JinJianQi @Ovilia This case shows how to use "mousewheel" to scroll single diectory: https://echarts.apache.org/examples/en/editor.html?c=custom-gantt-flight

A) Automatically turns zooming into scroll for touchpad events or B) Let the developer decide whether the scroll event should be handled to be zooming or scrolling?

In my opinion, that should be B, because what "mousewheel" stands for is probably decided by the app designer rather than users behaviors.

Does moveOnMouseWheel already match the requirements?

100pah avatar Sep 19 '22 14:09 100pah

@100pah @Ovilia As far as I know, the moveOnMouseWheel now just can't distinguish between X and Y directions. To put it simply, I just want to move both directions through touchPad and zoom in/out through separating two fingers (when we separate two fingers, it'll trigger wheel event and the mouseEvent.ctrlKey will be true).

Btw, when I set zoomOnMouseWheel to ctrl, I can actually zoom in/out through touchPad, but the speed is too fast.

In my commit, I just change the method of calculating the required offset, and I would like to set WindowScrollSpeedFactor as one of the options, so that users can change the speed when wheel.

jianqi-jin avatar Sep 20 '22 15:09 jianqi-jin

@JinJianQi @Ovilia I got your point. And the effect is good when both x and y direction can be controlled by two finger move and zoom by two fingers separating 👍 .

I have some suggestions about the option: as Ovilia mentioned, the option name gestureOnTouchPad is not straightforward enough and attached to "touchpad". I think there are three possible solutions:

  • A. Use natureMoveOnMouseWheel: true to represents: two finger move to scroll both x and y direction, and two finger separation to zoom in/out. And leave the behavior of the existing moveOnMouseWheel: true zoomOnMouseWheel: true not changed.
  • B. Use the existing moveOnMouseWheel: 'all' to represents: two finger move to scroll both x and y direction, and two finger separation to zoom in/out. And leave the behavior of the existing moveOnMouseWheel: true zoomOnMouseWheel: true not changed.
  • C. Use the existing moveOnMouseWheel: true to represents: two finger move to scroll both x and y direction, and two finger separation to zoom in/out. And drop the original behavior of moveOnMouseWheel: true (that is, only can scroll on one direction)

BTW, @JinJianQi, the code style should follow the style standard, for example, use 4 spaces indent rather than 2 spaces ~

100pah avatar Sep 21 '22 08:09 100pah

@100pah Thanks for your reply :)

From my side, I think both A and B are good, C just changes the original behavior, which means we may need another way to represent the original behavior.

In addition, for the style, I'll update this PR later ~

jianqi-jin avatar Sep 22 '22 15:09 jianqi-jin

Gentle ping 🙏 Was looking for this exact feature as well 😄 Thanks for the PR!

tihuan avatar Feb 02 '23 17:02 tihuan

This also works on Windows touchpads. The current scaling factor of ECharts is too large for Windows touchpads. The slightest pinch you make on the touchpad will cause the chart to scale dramatically. This PR solves this problem as well.

However, it looks like zoomOnMouseWheel: true is broken with gestureOnTouchPad: true. When I scroll the wheel, the chart moves instead of zooming. When I set gestureOnTouchPad: false, the chart scales as usual with the scroll wheel.

Chaoses-Ib avatar May 07 '23 08:05 Chaoses-Ib

Just wanted to drop a friendly message to check for any updates on this PR. The feature that has been added is exactly what we need for our project, and we're eagerly looking forward to its merge. Great work, by the way @JinJianQi 💯

masoudmanson avatar Jun 23 '23 16:06 masoudmanson

Sorry for the very late reply, and you guys are awesome.

@Chaoses-Ib I haven't found a suitable method to accurately differentiate actions from the trackpad and the mouse. From the wheel event perspective, vertical scrolling with two fingers on the trackpad is exactly the same as using the mouse wheel. So it seems to me that it's not possible to use the trackpad for movement and the mouse wheel for zooming (because, in my view, their wheel events are the same).

@100pah Since this parameter includes both move and zoom, it will definitely affect zoomOnMouseWheel and moveOnMouseWheel. My current approach is that if natureMoveOnMouseWheel is set to true, then ignore zoomOnMouseWheel and moveOnMouseWheel. If the user wants to use zoomOnMouseWheel and moveOnMouseWheel, they need to set natureMoveOnMouseWheel to false. Can you help me review this again? @100pah

jianqi-jin avatar Aug 22 '23 05:08 jianqi-jin

Since this PR has been opened for a long time. I'll close this and create a new one.

jianqi-jin avatar Dec 02 '23 19:12 jianqi-jin