feat(axis): provide splitLine.showMinLine and splitLine.showMaxLine. … #16947

MeetzhDing · 2022-04-27T06:19:10Z

Brief Information

This pull request is in the type of:

bug fixing
new feature
others

What does this PR do?

Resolves #16946

provide splitLine.showMinLine and splitLine.showMaxLine.

When using xAxis[type=time], it allow user hide first/last splitLine.

uplot style:

After: How is it fixed in this PR?

Like showMinLabel/showMaxLabel, it add new option showMinLine adn showMaxLine.

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

echarts-bot · 2022-04-27T06:19:12Z

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.

pissang · 2022-04-27T07:17:23Z

Please don't commit the files in the dist folder

src/coord/Axis.ts

src/component/axis/CartesianAxisView.ts

Ovilia · 2022-05-26T08:48:58Z

I have some concern on this PR.

I think it may be a better idea to hide min/max line only if the first/last tick is not calculated by nice tick (which usually happens when the user deliberately sets a min/max value to the axis.

For example, in this demo, the min tick is -20 and it is expected to be displayed because -20 is a nice tick.

While in another case, the axis is set to have min: -22. In this case, the splitLine of -22 is expected to be hidden if showMinLine is set to be false.

I think this logic may make more sense. @pissang What do you think?

MeetzhDing · 2022-07-01T08:38:01Z

@Ovilia @pissang
how to judge whether it is nice tick? Or is this any plan yet?

Ovilia · 2022-07-27T04:07:32Z

@MeetzhDing Sorry for the late reply. You can find the code here.

MeetzhDing · 2022-08-09T04:57:59Z

@Ovilia showMinLine type is boolean | 'auto' now

    // null/undefined (true) | true | false | 'auto' (true when nick tick only)
    showMinLine?: boolean | 'auto',
    // null/undefined (true) | true | false | 'auto' (true when nick tick only)
    showMaxLine?: boolean | 'auto',

src/component/axis/CartesianAxisView.ts

src/coord/axisCommonTypes.ts

MeetzhDing · 2022-09-30T04:22:43Z

@Ovilia please check pr again

MeetzhDing · 2022-10-28T08:02:02Z

@Ovilia please check pr again

Ovilia

Thanks for your contribution! I think the PR is close to the solution, only a few small tips.
Also, if you don't have some special reason, please use regular push instead of force push. Otherwise, the robot may not tag the PR to be revision state.

src/component/axis/CartesianAxisView.ts

Ovilia · 2022-11-08T04:16:02Z

src/component/axis/CartesianAxisView.ts

@@ -135,6 +136,30 @@ const axisElementBuilders: Record<typeof selfBuilderAttrs[number], AxisElementBu
            tickModel: splitLineModel
        });

+        const axisScale = axis.scale as IntervalScale;
+        if (ticksCoords.length > 2 && axisScale.getInterval) {


What if ticksCoords.length is 2 but the data range is not equal to the ticks' values?

When ticksCoords length is 2, the splitLine is not expected to be hidden

Why? For example, when data range is 11~19 and ticksCoords is [10, 20], I believe there should be no split line, right?

test/axis-interval.html

Ovilia · 2022-11-08T04:18:15Z

test/axis-interval.html

@@ -534,8 +540,68 @@ <h1>Test xAxis.axisTick.interval</h1>



+    <script>
+
+        require([


Please add more test cases to test boundary situations. You may start with the cases in my above questions.

Ovilia · 2022-11-08T04:18:36Z

src/component/axis/CartesianAxisView.ts

+            const interval = axisScale.getInterval() || null;
+
+            const showMinLine = splitLineModel.get('showMinLine');
+            if (showMinLine === false


!showMinLine should be better.

MeetzhDing · 2022-11-09T16:40:21Z

@Ovilia please check again

Ovilia

I've run visual tests and LGTM. @plainheart would you like to double-check this PR?

Ovilia · 2022-11-15T09:14:37Z

src/component/axis/CartesianAxisView.ts

@@ -135,6 +136,30 @@ const axisElementBuilders: Record<typeof selfBuilderAttrs[number], AxisElementBu
            tickModel: splitLineModel
        });

+        const axisScale = axis.scale as IntervalScale;
+        if (ticksCoords.length > 2 && axisScale.getInterval) {


Why? For example, when data range is 11~19 and ticksCoords is [10, 20], I believe there should be no split line, right?

…6946

echarts-bot · 2022-11-15T10:30:29Z

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.

plainheart · 2024-07-18T05:23:30Z

Superseded by #20114

pull-request-size bot added the size/XXL label Apr 27, 2022

echarts-bot bot added PR: awaiting review PR: first-time contributor labels Apr 27, 2022

pissang reviewed Apr 27, 2022

View reviewed changes

src/coord/Axis.ts Outdated Show resolved Hide resolved

MeetzhDing force-pushed the fix-16946 branch from 8125698 to b646e9e Compare April 27, 2022 07:31

pull-request-size bot added size/S and removed size/XXL labels Apr 27, 2022

pissang added this to the 5.4 milestone Apr 27, 2022

pissang reviewed Apr 29, 2022

View reviewed changes

src/component/axis/CartesianAxisView.ts Outdated Show resolved Hide resolved

MeetzhDing force-pushed the fix-16946 branch from b646e9e to e722624 Compare April 29, 2022 03:35

Ovilia added PR: revision needed and removed PR: awaiting review labels Jul 27, 2022

MeetzhDing dismissed a stale review via 633c6f0 August 9, 2022 04:51

MeetzhDing force-pushed the fix-16946 branch from e722624 to 633c6f0 Compare August 9, 2022 04:51

echarts-bot bot removed the PR: revision needed label Aug 9, 2022

pull-request-size bot removed the size/S label Aug 9, 2022

echarts-bot bot added the PR: awaiting review label Aug 9, 2022

pull-request-size bot added the size/M label Aug 9, 2022

Ovilia modified the milestones: 5.4, 5.4.1 Sep 1, 2022

Ovilia requested a review from pissang September 2, 2022 07:48

Ovilia requested changes Sep 23, 2022

View reviewed changes

src/component/axis/CartesianAxisView.ts Outdated Show resolved Hide resolved

src/component/axis/CartesianAxisView.ts Outdated Show resolved Hide resolved

src/coord/axisCommonTypes.ts Outdated Show resolved Hide resolved

echarts-bot bot added PR: revision needed and removed PR: awaiting review labels Sep 23, 2022

MeetzhDing force-pushed the fix-16946 branch from 633c6f0 to a31e928 Compare September 30, 2022 04:22

feat(axis): provide splitLine.showMinLine and splitLine.showMaxLine

8be4e47

MeetzhDing force-pushed the fix-16946 branch from a31e928 to 8be4e47 Compare September 30, 2022 04:25

Ovilia requested changes Nov 8, 2022

View reviewed changes

feat(axis): provide splitLine.showMinLine and splitLine.showMaxLine

7d34f5c

echarts-bot bot added PR: awaiting review and removed PR: revision needed labels Nov 9, 2022

Ovilia reviewed Nov 15, 2022

View reviewed changes

Merge branch 'master' of /~https://github.com/apache/echarts into fix-1…

075741c

…6946

echarts-bot bot added the PR: awaiting doc Document changes is required for this PR. label Nov 15, 2022

plainheart modified the milestones: 5.4.1, 5.5.0 Nov 20, 2022

Ovilia modified the milestones: 5.5.0, 5.5.1 Jan 18, 2024

Ovilia modified the milestones: 5.5.1, 5.5.2 Jun 28, 2024

plainheart mentioned this pull request Jul 18, 2024

feat(axis): add showMinLine / showMaxLine option for splitLine #20114

Merged

8 tasks

plainheart removed this from the 5.5.2 milestone Jul 18, 2024

plainheart closed this Jul 18, 2024

echarts-bot bot removed the PR: awaiting review label Jul 18, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(axis): provide splitLine.showMinLine and splitLine.showMaxLine. … #16947

feat(axis): provide splitLine.showMinLine and splitLine.showMaxLine. … #16947

MeetzhDing commented Apr 27, 2022 •

edited by plainheart

Loading

echarts-bot bot commented Apr 27, 2022

pissang commented Apr 27, 2022

Ovilia commented May 26, 2022

MeetzhDing commented Jul 1, 2022

Ovilia commented Jul 27, 2022

MeetzhDing commented Aug 9, 2022

MeetzhDing commented Sep 30, 2022

MeetzhDing commented Oct 28, 2022

Ovilia left a comment •

edited

Loading

Ovilia Nov 8, 2022

MeetzhDing Nov 9, 2022

Ovilia Nov 15, 2022

Ovilia Nov 8, 2022

Ovilia Nov 8, 2022

MeetzhDing commented Nov 9, 2022

Ovilia left a comment

Ovilia Nov 15, 2022

echarts-bot bot commented Nov 15, 2022

plainheart commented Jul 18, 2024

		@@ -534,8 +540,68 @@ <h1>Test xAxis.axisTick.interval</h1>



		<script>

		require([

feat(axis): provide splitLine.showMinLine and splitLine.showMaxLine. … #16947

feat(axis): provide splitLine.showMinLine and splitLine.showMaxLine. … #16947

Conversation

MeetzhDing commented Apr 27, 2022 • edited by plainheart Loading

Brief Information

What does this PR do?

After: How is it fixed in this PR?

Document Info

echarts-bot bot commented Apr 27, 2022

pissang commented Apr 27, 2022

Ovilia commented May 26, 2022

MeetzhDing commented Jul 1, 2022

Ovilia commented Jul 27, 2022

MeetzhDing commented Aug 9, 2022

MeetzhDing commented Sep 30, 2022

MeetzhDing commented Oct 28, 2022

Ovilia left a comment • edited Loading

Choose a reason for hiding this comment

Ovilia Nov 8, 2022

Choose a reason for hiding this comment

MeetzhDing Nov 9, 2022

Choose a reason for hiding this comment

Ovilia Nov 15, 2022

Choose a reason for hiding this comment

Ovilia Nov 8, 2022

Choose a reason for hiding this comment

Ovilia Nov 8, 2022

Choose a reason for hiding this comment

MeetzhDing commented Nov 9, 2022

Ovilia left a comment

Choose a reason for hiding this comment

Ovilia Nov 15, 2022

Choose a reason for hiding this comment

echarts-bot bot commented Nov 15, 2022

plainheart commented Jul 18, 2024

MeetzhDing commented Apr 27, 2022 •

edited by plainheart

Loading

Ovilia left a comment •

edited

Loading