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

Implement DataZoom Feature - Fixes #19732 #19835

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

Conversation

WZY009
Copy link

@WZY009 WZY009 commented Apr 15, 2024

Overview

This Pull Request introduces a new mobile-specific feature for the dataZoom component within ECharts. It resolves feature #19732, which requests the implementation of mobile dataZoom-Inside for horizontal and vertical using touch gestures and scrolling scale using mouse.

Changes

The following are the key changes made in this PR:

  • Enhanced InsideZoomModel.ts and InsideZoomView.ts to support touch gesture recognition for mobile devices.
  • Integrated new gesture handling logic that distinguishes between horizontal and vertical gestures to apply data zoom accordingly.

Testing

Manual testing was conducted to confirm the functionality. Test file : test/test-scroll-zoom.html

By merging this PR, ECharts will provide an enhanced interactive experience for mobile users, allowing more precise control over data visualization.

Screenshots

image

Notes

  • Additional testing on a wider range of devices is recommended to ensure universal compatibility and responsiveness.
  • Further feedback from the community can be valuable for iterative improvement of the touch interaction model.

Resolves: #19732

Copy link

echarts-bot bot commented Apr 15, 2024

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.

To reviewers: If this PR is going to be described in the changelog in the future release, please make sure this PR has one of the following labels: PR: doc ready, PR: awaiting doc, PR: doc unchanged

This message is shown because the PR description doesn't contain the document related template.

Copy link
Contributor

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

@admsev
Copy link

admsev commented May 25, 2024

@gooroodev review

@gooroodev
Copy link

Summary of Changes

  1. .eslintrc-common.yaml:

    • Added several global variables for browser environment.
    • Set environment configurations for browser, node, and ES2021.
  2. InsideZoomModel.ts:

    • Added new options zoomXOnMouseWheel and zoomYOnMouseWheel to control zooming on X and Y axes separately.
    • Updated the default options to include these new options.
  3. InsideZoomView.ts:

    • Introduced private properties lastX and lastY to track the last mouse positions.
    • Added methods handleZoom, handlePan, and handleScrollMove to handle zooming and panning logic.
    • Enhanced the scrollMove handler to support new zooming functionality.
  4. test-scroll-zoom.html:

    • Created a new HTML test file to test the scroll zoom feature with a sample chart.
  5. tsconfig.json:

    • Updated the TypeScript target from ES3 to ES6.

Issues, Bugs, and Typos

  1. Typo in InsideZoomModel.ts:

    • Line 23: configration should be configuration.
  2. Typo in test-scroll-zoom.html:

    • Line 63: Catergory should be Category.
  3. Missing Newline at End of File:

    • The file InsideZoomModel.ts is missing a newline at the end of the file.

Proposed Improvements

Improvement 1:

  • Correct the typo in InsideZoomModel.ts.

Improvement 2:

  • Correct the typo in test-scroll-zoom.html.

Improvement 3:

  • Add a newline at the end of InsideZoomModel.ts.

General Review of Code Quality and Style

  • Code Quality: The code appears to be well-structured and follows consistent naming conventions. The addition of new features is well-integrated into the existing codebase.
  • Code Style: The code adheres to standard TypeScript and JavaScript practices. Proper use of types and interfaces is maintained.
  • Documentation: The added comments and JSDoc-like annotations are helpful for understanding the new features and their configurations.
  • Testing: A new test HTML file is provided to test the new scroll zoom feature, which is a good practice to ensure functionality.

Detailed Code Review

Improvement 1:

+    /**
+     * Add configuration attribute to control zoom X and Y axis.
+     */

Improvement 2:

+                xAxisData.push('Category' + i);

Improvement 3:

 export default InsideZoomModel;
\ No newline at end of file
+ 

These improvements address the identified issues and enhance the overall quality and correctness of the code.

Yours, Gooroo.dev. To receive reviews automatically, install Github App

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