fix(marker): `markArea` of bar series now covers whole specified categories #17098

jiawulin001 · 2022-05-24T07:42:59Z

Brief Information

This pull request is in the type of:

bug fixing
new feature
others

What does this PR do?

MarkArea in bar series/category axis now covers the categories specified rather than area between bars of specified categories.

Fixed issues

Details

Before: What was the problem?

MarkArea in barSeries/category-axis is marking the area between bars of category specified, which is not wanted in category axis chart.

#3573
#12341
--	--
#17021

After: How is it fixed in this PR?

Now markArea marks the whole category zones specified. Multi-bars and invert input are both considered.

Left MarkArea

markArea: {
          data: [
            [
              {
                name: `Invert input`,
                xAxis: 'Thu'
              },
              {
                xAxis: 'Mon'
              }
            ]
          ]
        }

Left MarkArea

markArea: {
          data: [
            [
              {
                name: 'Single Emphasis',
                xAxis: 'Sun'
              },
              {
                xAxis: 'Sun'
              }
            ]
          ]
        }

Misc

The API has been changed (apache/echarts-doc#xxx).
This PR depends on ZRender changes (ecomfe/zrender#xxx).

Related test cases or examples to use the new APIs

bar-markArea.html

Others

Merging options

Please squash the commits into a single one when merging.

Other information

echarts-bot · 2022-05-24T07:43:01Z

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.

dynamikus · 2022-05-27T15:01:25Z

Any plans to merge this pr?

Ovilia · 2022-05-29T15:20:54Z

@dynamikus Thanks for your contribution and please be patient to wait for reviews since we are mostly working part-time on the project. :)

Ovilia · 2022-05-29T15:25:20Z

This PR looks awesome! I would like to see more test cases, for example, when xAxis and yAxis are both configured in the data, or x and y values.

dynamikus · 2022-05-30T22:25:20Z

@jiawulin001
Thank you for looking into this. Sorry if my questions sounded condescending or demanding that was never my attention.
Keep up with the great work 👍

jiawulin001 · 2022-06-07T01:35:00Z

I would like to see more test cases

@Ovilia More test cases are on the way.

Sorry if my questions sounded condescending or demanding that was never my attention.

@dynamikus Thank you for your applause, and it's totally fine to ask. I am really glad if my work can help someone else.

…#17021

jiawulin001 · 2022-06-07T08:39:47Z

@Ovilia A new test case in the same html is added

Ovilia · 2022-06-08T06:21:40Z

@jiawulin001 Thanks! Please also run the visual test for all cases and see if there are diffs that we should be aware of. You should compare local changes with the nightly build of master branch.

jiawulin001 · 2022-06-13T01:54:33Z

@Ovilia Thank you for guiding me to visual test. However since "Run all test" takes forever to run, I run the test relative to markArea and markLine and the result is as followed. The first test case is added by me, so it's expected to see differences there and no other test cases are affected.

Update 2022/6/13 11:28

Seems that there are issues of MarkPoint caused by changes, dealing with it.

Update 2022/6/13 11:39

Issue resolved, now most visual test passed. Let me know if I need to do other testcases.

Also it's very time consuming to run all visual tests. It takes 40+ minutes to execute all of them. Is there a way to solve or improve this?

Ovilia · 2022-06-13T08:43:45Z

@jiawulin001 Thanks for running the visual tests! I think you are working with the unexpected test results, right?

Also it's very time consuming to run all visual tests. It takes 40+ minutes to execute all of them. Is there a way to solve or improve this?

Usually, we only do a thorough test before releasing a new version or the PR changes a lot of logic or the reviewers are not very confident with the changes. For PRs like this, test the files that contain "mark" in their names should be enough. But a thorough test is always helpful so thanks for the effort and time! :)

jiawulin001 · 2022-06-13T09:13:34Z

I think you are working with the unexpected test results, right?

If you mean the issues of markPoint, it's been resolved in the update. No other issues occur.

plainheart · 2022-06-14T06:00:32Z

Hi @jiawulin001, Thanks for your recent contributions! They help a lot! It would be better if you could check the reason why the configured husky hook and ESLint plugin don't work for you. It can give you on-time corresponding warnings and fixing strategies and reduce extra commits that fix lint issues after you make them work normally. :)

jiawulin001 · 2022-06-14T08:39:22Z

My apologies for those lint fix commits! I've been using github desktop to submit commits and the hooks seem not to work on it. I'll use my IDE to submit commits and make sure I execute lint scripts before them in the future. Thank you for your reminding!

jiawulin001 · 2022-06-14T08:51:53Z

I have another question regarding visual test. When I am doing all kinds of PR, is there a criterion how I pick what visual tests to do and how should I present the results to the community?

Ovilia · 2022-06-14T09:02:23Z

I have another question regarding visual test. When I am doing all kinds of PR, is there a criterion how I pick what visual tests to do and how should I present the results to the community?

If you want to export the report, there's a "ALL RUNS" button on the menu. Check the versions and time to know which running should be exported and click "report" to export.

SevenOutman · 2022-07-20T09:51:19Z

Any update on this PR? Would help a lot as neither markArea or splitArea is working correctly today in bar charts with dataZoom :(

jiawulin001 · 2022-07-23T03:38:06Z

@Ovilia Hi, I think I've resolved the conflict in merging. Any other suggestions on how should I improve this PR? Or should we move forward to merging?

ysldnadotme · 2022-09-19T09:57:31Z

Hi, Is there any plan to merge the problem to the latest version in the near future? @Ovilia 🤔️

Ovilia · 2022-09-23T04:36:55Z

Hi, Is there any plan to merge the problem to the latest version in the near future? @Ovilia 🤔️

This PR is scheduled in 5.4.1, which should be published in November.

Ovilia

I've run with all test cases and it looks fine.

echarts-bot · 2022-09-23T05:37:15Z

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.

echarts-bot · 2022-09-23T05:37:29Z

Congratulations! Your PR has been merged. Thanks for your contribution! 👍

…17098)

andrius-kurtinaitis · 2022-12-28T08:23:15Z

It seems this PR breaks zoom functionality: #18099

ily-salamat · 2023-01-02T10:10:00Z

Since version 5.4.1 the markArea in my bar chart is broken, I think it's related to this PR

plainheart · 2023-01-02T10:15:29Z

@ily-salamat Hi, thanks for your report! Please create a new issue and make a reference to this PR. We'll try to confirm and fix it in v5.5.0.

plainheart · 2023-01-02T10:21:59Z

Also, thank you @andrius-kurtinaitis for filing a similar bug. I've marked your issue as high-priority and will fix it first.

fix: markArea of bar series now covers whole category

a051e30

pull-request-size bot added the size/M label May 24, 2022

echarts-bot bot added the PR: awaiting review label May 24, 2022

fix lint error

27c69b0

pissang added this to the 5.4 milestone May 28, 2022

jiawulin001 added 2 commits June 7, 2022 09:40

Merge branch 'master' of https://github.com/apache/echarts into issue…

8b8e166

…#17021

New testcase added

6dbd5df

fix: markPoint now can show normally

b47d056

jiawulin001 dismissed a stale review via b47d056 June 13, 2022 03:50

fix lint error

5186308

chore: resolve conflict

70db7c6

Ovilia requested review from 100pah and pissang August 9, 2022 04:04

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

Ovilia approved these changes Sep 23, 2022

View reviewed changes

echarts-bot bot removed the PR: awaiting review label Sep 23, 2022

Ovilia merged commit 583711e into apache:master Sep 23, 2022

Ovilia added the PR: doc unchanged label Sep 23, 2022

plainheart mentioned this pull request Sep 25, 2022

style(lint): fix spacing issues and remove unused imports and functions #17695

Merged

8 tasks

Ovilia added the PR: author is committer label Sep 28, 2022

plainheart added a commit that referenced this pull request Nov 8, 2022

style: fix code style flaw and remove unnecessary default parameter. (#…

b9a19eb

…17098)

plainheart mentioned this pull request Nov 8, 2022

style: fix code style flaw and remove unnecessary default parameter. #17887

Merged

8 tasks

plainheart added a commit that referenced this pull request Nov 8, 2022

style: fix code style flaw and remove unnecessary default parameter. (#…

63dd98a

…17098)

plainheart changed the title ~~fix: markArea of bar series now covers whole categories specified~~ fix(marker): markArea of bar series now covers whole specified categories Nov 30, 2022

ily-salamat mentioned this pull request Jan 2, 2023

[Bug] MarkArea is broken in version 5.4.1 #18130

Closed

plainheart mentioned this pull request Jan 10, 2023

[Bug] Pie chart, add 'markLine' to 'series' , report an error 在饼图里使用markLine属性会报错 #18150

Closed

Ovilia mentioned this pull request Feb 2, 2023

fix(markArea): markArea range in bar series #18130 #18229

Merged

8 tasks

plainheart mentioned this pull request Jul 12, 2023

[Bug] MarkArea not fully fixed in version 5.4.2 #18499

Open

X-oss-byte mentioned this pull request May 25, 2024

[Snyk] Upgrade echarts from 5.4.1 to 5.5.0 X-oss-byte/mempool#84

Open

alexdohm mentioned this pull request Oct 24, 2024

[Bug] MarkArea in Bar Chart still does not fill area behind bars #20450

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(marker): `markArea` of bar series now covers whole specified categories #17098

fix(marker): `markArea` of bar series now covers whole specified categories #17098

jiawulin001 commented May 24, 2022

echarts-bot bot commented May 24, 2022

dynamikus commented May 27, 2022

Ovilia commented May 29, 2022

Ovilia commented May 29, 2022

dynamikus commented May 30, 2022

jiawulin001 commented Jun 7, 2022 •

edited

Loading

jiawulin001 commented Jun 7, 2022

Ovilia commented Jun 8, 2022 •

edited

Loading

jiawulin001 commented Jun 13, 2022 •

edited

Loading

Ovilia commented Jun 13, 2022

jiawulin001 commented Jun 13, 2022

plainheart commented Jun 14, 2022

jiawulin001 commented Jun 14, 2022

jiawulin001 commented Jun 14, 2022

Ovilia commented Jun 14, 2022

SevenOutman commented Jul 20, 2022

jiawulin001 commented Jul 23, 2022

ysldnadotme commented Sep 19, 2022

Ovilia commented Sep 23, 2022

Ovilia left a comment

echarts-bot bot commented Sep 23, 2022

echarts-bot bot commented Sep 23, 2022

andrius-kurtinaitis commented Dec 28, 2022

ily-salamat commented Jan 2, 2023

plainheart commented Jan 2, 2023

plainheart commented Jan 2, 2023

fix(marker): markArea of bar series now covers whole specified categories #17098

fix(marker): markArea of bar series now covers whole specified categories #17098

Conversation

jiawulin001 commented May 24, 2022

Brief Information

What does this PR do?

Fixed issues

Details

Before: What was the problem?

After: How is it fixed in this PR?

Left MarkArea

Left MarkArea

Misc

Related test cases or examples to use the new APIs

Others

Merging options

Other information

echarts-bot bot commented May 24, 2022

dynamikus commented May 27, 2022

Ovilia commented May 29, 2022

Ovilia commented May 29, 2022

dynamikus commented May 30, 2022

jiawulin001 commented Jun 7, 2022 • edited Loading

jiawulin001 commented Jun 7, 2022

Ovilia commented Jun 8, 2022 • edited Loading

jiawulin001 commented Jun 13, 2022 • edited Loading

Update 2022/6/13 11:28

Update 2022/6/13 11:39

Ovilia commented Jun 13, 2022

jiawulin001 commented Jun 13, 2022

plainheart commented Jun 14, 2022

jiawulin001 commented Jun 14, 2022

jiawulin001 commented Jun 14, 2022

Ovilia commented Jun 14, 2022

SevenOutman commented Jul 20, 2022

jiawulin001 commented Jul 23, 2022

ysldnadotme commented Sep 19, 2022

Ovilia commented Sep 23, 2022

Ovilia left a comment

Choose a reason for hiding this comment

echarts-bot bot commented Sep 23, 2022

echarts-bot bot commented Sep 23, 2022

andrius-kurtinaitis commented Dec 28, 2022

ily-salamat commented Jan 2, 2023

plainheart commented Jan 2, 2023

plainheart commented Jan 2, 2023

fix(marker): `markArea` of bar series now covers whole specified categories #17098

fix(marker): `markArea` of bar series now covers whole specified categories #17098

jiawulin001 commented Jun 7, 2022 •

edited

Loading

Ovilia commented Jun 8, 2022 •

edited

Loading

jiawulin001 commented Jun 13, 2022 •

edited

Loading