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: allow areaStyle.origin to take number as input #16719

Merged
merged 2 commits into from
Mar 24, 2022

Conversation

jiawulin001
Copy link
Member

Brief Information

This pull request is in the type of:

  • bug fixing
  • new feature
  • others

What does this PR do?

Allow series-line.areaStyle.origin to take number as input

Fixed issues

Details

Before: What was the problem?

The choices for series-line.areaStyle.origin for now is limited to 'start', 'end', and 'auto', which correspond to baseline at bottom, top or axis of the view.

  • origin: 'start'

eChIssue#16717-2

  • origin: 'end'

eChIssue#16717-3

  • origin: 'auto'

eChIssue#16717-1

  • And now what is required is to let origin accept a number and set the baseline right at the specified level. Take the picture below as an exmaple.
    eChIssue#16717-4

After: How is it fixed in this PR?

  • What is changed in code?
  • Here a second else if is added to take number in and set it to valueStart. NaN is excluded from such situation and will be treated as undefined.
function getValueStart(valueAxis, valueOrigin) {
      var valueStart = 0;
      var extent = valueAxis.scale.getExtent();

      if (valueOrigin === 'start') {
        valueStart = extent[0];
      } else if (valueOrigin === 'end') {
        valueStart = extent[1];
      } // If origin is specified as a number, use it as
      // valueStart directly
      else if (isNumber(valueOrigin) && !isNaN(valueOrigin)) {
          valueStart = valueOrigin;
        } // auto
        else {
            // Both positive
            if (extent[0] > 0) {
              valueStart = extent[0];
            } // Both negative
            else if (extent[1] < 0) {
                valueStart = extent[1];
              } // If is one positive, and one negative, onZero shall be true

          }

      return valueStart;
    }
  • An example of using number as areaStyle.origin

eChIssue#16717-6

Misc

  • The API has been changed (apache/echarts-doc#xxx).

  • This PR depends on ZRender changes (ecomfe/zrender#xxx).

  • The echarts-doc has not been changed yet, I can submit another PR to update the doc.

@echarts-bot
Copy link

echarts-bot bot commented Mar 22, 2022

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.

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.

@pissang pissang added this to the 5.3.2 milestone Mar 22, 2022
@pissang
Copy link
Contributor

pissang commented Mar 22, 2022

@jiawulin001 Wow looks great!

BTW do you mind adding a test case?

@jiawulin001
Copy link
Member Author

yes, I can add a test case

@jiawulin001
Copy link
Member Author

jiawulin001 commented Mar 23, 2022

@pissang Any suggestion on which test.ts to add the test case to? I didn't find any existent test file that includes areaStyle, so my plan is to add a new test file under test/ut/spec/series. Is that OK?

@pissang
Copy link
Contributor

pissang commented Mar 23, 2022

@jiawulin001 We usually add test cases in the test folder. Run npm run mktest area-origin and it will generate a template for you.

@jiawulin001
Copy link
Member Author

@pissang Hi, the test case has been added. Check it out!

@pissang pissang merged commit ad3a07a into apache:master Mar 24, 2022
@echarts-bot
Copy link

echarts-bot bot commented Mar 24, 2022

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

@plainheart
Copy link
Member

Hi @jiawulin001, thanks for your contribution to this new feature and this is just a reminder. Are you working on adding or planning to add this new area origin value for the line series in the documentation?

@jiawulin001
Copy link
Member Author

@plainheart Oh yes, thank you for your reminding! The PR for updating documentation has been submitted.

@plainheart plainheart removed the PR: awaiting doc Document changes is required for this PR. label Mar 28, 2022
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.

[Feature] Area Chart with limit line where area fill is on both sides of mark line.
3 participants