Skip to content

Fix 12109#12418

Merged
Ovilia merged 20 commits intoapache:masterfrom
gracelia:fix-12109
May 28, 2020
Merged

Fix 12109#12418
Ovilia merged 20 commits intoapache:masterfrom
gracelia:fix-12109

Conversation

@gracelia
Copy link
Copy Markdown
Contributor

@gracelia gracelia commented Apr 12, 2020

Brief Information

This pull request is in the type of:

  • bug fixing
  • new feature
  • others

What does this PR do?

correct bar polar rendering negative values

Fixed issues

#12109

Details

Before: What was the problem?

image

After: How is it fixed in this PR?

image

Usage

Are there any API changes?

  • The API has been changed.

Related test cases or examples to use the new APIs

NA.

Others

Merging options

  • Please squash the commits into a single one when merge.

Other information

@echarts-bot
Copy link
Copy Markdown

echarts-bot bot commented Apr 12, 2020

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.

Ovilia
Ovilia requested changes Apr 14, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@Ovilia Ovilia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your contribution. The basic idea is all right but some modification is required.

@pissang pissang added this to the 4.9.0 milestone Apr 15, 2020
@gracelia
Copy link
Copy Markdown
Contributor Author

gracelia commented Apr 20, 2020

  1. remove min max calculation logic with axis.scale.getExtent()
  2. moved cases to bar-polar-stack.html
  3. add more cases

@susiwen8
Copy link
Copy Markdown
Contributor

susiwen8 commented Apr 20, 2020

Did you set correct user.name and user.email?

@gracelia
Copy link
Copy Markdown
Contributor Author

gracelia commented Apr 20, 2020

Did you set correct user.name and user.email?

Sorry, not quite sure of the problem.

@susiwen8
Copy link
Copy Markdown
Contributor

susiwen8 commented Apr 20, 2020

I mean the user.name and user.email of you git. It seems like you didn't use your github account to push commit

@gracelia
Copy link
Copy Markdown
Contributor Author

gracelia commented Apr 21, 2020

I mean the user.name and user.email of you git. It seems like you didn't use your github account to push commit

I got your point. Should I change account and commit again?

@susiwen8
Copy link
Copy Markdown
Contributor

susiwen8 commented Apr 21, 2020

I mean the user.name and user.email of you git. It seems like you didn't use your github account to push commit

I got your point. Should I change account and commit again?

That won't be necessary, Squash all commits then merge can do the job. I think.

@gracelia
Copy link
Copy Markdown
Contributor Author

gracelia commented Apr 21, 2020

I mean the user.name and user.email of you git. It seems like you didn't use your github account to push commit

I got your point. Should I change account and commit again?

That won't be necessary, Squash all commits then merge can do the job. I think.

Thanks, I'll make a try.

@susiwen8
Copy link
Copy Markdown
Contributor

susiwen8 commented Apr 21, 2020

I mean the user.name and user.email of you git. It seems like you didn't use your github account to push commit

I got your point. Should I change account and commit again?

That won't be necessary, Squash all commits then merge can do the job. I think.

Thanks, I'll make a try.

Or you can force push.

gracelia added 2 commits April 21, 2020 15:25
@gracelia
fix: 12109
7da4b5e 
fix: 12109, add test case

Revert "fix: 12109 add test case"

This reverts commit 66f5c04.

fix: 12109, add case in issue
@gracelia
Merge branch 'fix-12109' of https://github.com/BonGrace/incubator-ech…
047a9ba 
…arts into fix-12109
@gracelia
Copy link
Copy Markdown
Contributor Author

gracelia commented Apr 21, 2020 •
edited
Loading

I mean the user.name and user.email of you git. It seems like you didn't use your github account to push commit

I got your point. Should I change account and commit again?

That won't be necessary, Squash all commits then merge can do the job. I think.

Thanks, I'll make a try.

Or you can force push.

Do you know where to see if I had done right?

@susiwen8
Copy link
Copy Markdown
Contributor

susiwen8 commented Apr 21, 2020

Screen Shot 2020-04-21 at 16 21 20
Screen Shot 2020-04-21 at 16 21 29

@gracelia
Copy link
Copy Markdown
Contributor Author

gracelia commented Apr 22, 2020

@susiwen8 Clear, thanks.

Ovilia
Ovilia requested changes Apr 24, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@Ovilia Ovilia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some test cases don't have expected result.

Please compare with bar under grid: (change the data and min max)

option = {
    xAxis: {
        data: ['A', 'B', 'C']
    },
    yAxis: {
        min: 1,
        max: 6
    },
    series: [{
        name: '模拟数据',
        type: 'bar',
        stack: 'a',
        data: [2, 2, 2]
    }, {
        stack: 'a',
        type: 'bar',
        data: [2, 2, 2]
    }, {
        stack: 'a',
        type: 'bar',
        data: [2, 2, 2]
    }]
};

Ovilia
Ovilia requested changes May 13, 2020
View reviewed changes
Ovilia
Ovilia requested changes May 27, 2020
View reviewed changes
pissang
pissang previously approved these changes May 28, 2020
View reviewed changes
@Ovilia Ovilia merged commit 184d935 into apache:master May 28, 2020
@echarts-bot
Copy link
Copy Markdown

echarts-bot bot commented May 28, 2020

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

@Ovilia Ovilia mentioned this pull request May 29, 2020
Closed
5 tasks
@BillYu811 BillYu811 mentioned this pull request Apr 6, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Ovilia Ovilia Ovilia approved these changes

@pissang pissang pissang left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

4.9.0

Development

Successfully merging this pull request may close these issues.

4 participants

@gracelia @susiwen8 @Ovilia @pissang