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

feat(axis): custom axis tick/label positions #19919

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Conversation

Ovilia
Copy link
Contributor

@Ovilia Ovilia commented May 10, 2024

Brief Information

This pull request is in the type of:

  • bug fixing
  • new feature
  • others

What does this PR do?

Add option customValues to axisTick and axisLabel, which allow the user to specify tick/label positions.

This PR is based on #13636 . Since it's a long time since the opening of this PR, I made a few adjustment based on the original commit, with full regards to the contribution to @dvdkon 's contribution.

Fixed issues

#13627

Details

Before: What was the problem?

See issue #13627

After: How is it fixed in this PR?

The new option allows for completely custom tick/label positions, for example:

xAxis: {
	type: 'value',
	axisLabel: {
		customValues: [0, 4, 7, 8, 9]
	},
	axisTick: {
		alignWithLabel: true,
		customValues: [0, 0.5, 1, 1.5, 2, 8, 9]
	},
}

example

Usage

Are there any API changes?

  • The API has been changed.

In axis:

  • axisTick.customValues: (string | number | Date)[]
  • axisLabel.customValues: (string | number | Date)[]

Array of axis values on which a tick/label will be present (automatic tick generation is disabled)

Related test cases or examples to use the new APIs

test/axis-customTicks.html

Copy link

echarts-bot bot commented May 10, 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.

The pull request is marked to be PR: author is committer because you are a committer of this project.

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.

@Ovilia Ovilia added this to the 5.5.1 milestone May 10, 2024
Copy link
Contributor

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

@admsev
Copy link

admsev commented May 25, 2024

@gooroodev review

@gooroodev
Copy link

Summary of Changes

  1. Added Custom Tick/Label Positions:

    • Introduced customValues property to AxisTickOption and AxisLabelBaseOption interfaces in axisCommonTypes.ts.
    • Implemented logic to handle custom tick/label positions in axisTickLabelBuilder.ts by adding tickValuesToNumbers function and modifying createAxisLabels and createAxisTicks functions.
  2. Test Case:

    • Created a new HTML test file axis-customTicks.html to demonstrate and verify the functionality of custom tick/label positions for different axis types (value, category, time, log).

Issues, Bugs, and Typos

  1. Comma at the End of Object Properties:

    • In TypeScript interfaces, trailing commas are inconsistent. For example:
      customValues?: (number | string | Date)[]
      should be:
      customValues?: (number | string | Date)[];
  2. Sorting and Duplicating Ticks for Time Axis:

    • The logic to duplicate the first and last tick values for time axis might be confusing without comments explaining why it's necessary. Consider adding comments for clarity.

Proposed Improvements

  1. Consistency in Trailing Commas:

    • Ensure all properties in interfaces end with a comma for consistency.
      interface AxisTickOption {
          inside?: boolean;
          length?: number;
          lineStyle?: LineStyleOption;
          customValues?: (number | string | Date)[];
      }
  2. Enhanced Comments for Clarity:

    • Add comments to explain the purpose of duplicating first and last tick values for time axis.
      function tickValuesToNumbers(axis: Axis, values: (number | string | Date)[]) {
          const nums = zrUtil.map(values, val => axis.scale.parse(val));
          if (axis.type === 'time' && nums.length > 0) {
              // Time axis needs duplicate first/last tick to ensure proper rendering.
              // The first and last tick/label don't get drawn.
              nums.sort();
              nums.unshift(nums[0]);
              nums.push(nums[nums.length - 1]);
          }
          return nums;
      }

General Review of Code Quality and Style

  1. Modularity and Separation of Concerns:

    • The new functionality is well-integrated into existing functions without disrupting existing logic, demonstrating good modularity and separation of concerns.
  2. Code Readability:

    • The code is generally readable and follows good naming conventions. However, adding more comments, especially for complex logic, would enhance understanding.
  3. Test Coverage:

    • The new test file axis-customTicks.html provides comprehensive test cases for value, category, time, and log axes, ensuring the new feature is well-tested.
  4. Use of Utilities:

    • The use of zrUtil.map for mapping values is appropriate and leverages existing utilities effectively.

Summary

Overall, the pull request introduces a valuable feature for customizing axis tick/label positions. The code is well-structured, and the changes are integrated seamlessly. Minor improvements in consistency and comments would further enhance the quality and maintainability 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
Development

Successfully merging this pull request may close these issues.

None yet

4 participants