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

FIX #11176 xAxis.axisTick.interval appears different between echart4 and echart3.8 #11186

Merged
merged 7 commits into from Oct 18, 2019
Merged

Conversation

foolzhang
Copy link
Contributor

@foolzhang foolzhang commented Sep 8, 2019

different display in version 3.6 and 4 with xAxis.axisTick.interval

@foolzhang foolzhang changed the title fix bug #11176 bug FIX #11176 #bug Sep 8, 2019
@pissang
Copy link
Contributor

pissang commented Sep 8, 2019

@foolzhang
Copy link
Contributor Author

foolzhang commented Sep 8, 2019

how about this time @pissang

@pissang pissang requested review from 100pah and pissang Sep 9, 2019
@pissang pissang added this to the 4.5.0 milestone Sep 17, 2019
@pissang
Copy link
Contributor

pissang commented Sep 17, 2019

Scheduled in the version 4.5.0

@pissang pissang changed the title FIX #11176 #bug FIX #11176 xAxis.axisTick.interval appears different between echart4 and echart3.8 Sep 23, 2019
@@ -315,7 +320,10 @@ function fixOnBandTicksCoords(axis, ticksCoords, tickCategoryInterval, alignWith
ticksItem.coord -= shift / ((tickCategoryInterval + 1) * 2);
}
});
last = {coord: ticksCoords[ticksLen - 1].coord + shift};

diffSize = tickLen - ticksCoords[ticksLen - 1].tickValue;
Copy link
Contributor

@pissang pissang Oct 15, 2019

Choose a reason for hiding this comment

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

I think this should be

diffSize = tickLen - crossLen;

Copy link
Contributor Author

@foolzhang foolzhang Oct 16, 2019

Choose a reason for hiding this comment

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

@pissang thanks your time , i am feel sorry about not consider dataZoom , but if like this

Suggested change
diffSize = tickLen - ticksCoords[ticksLen - 1].tickValue;
diffSize = diffSize = tickLen - crossLen;

in my test demo axis-interval.html

{
        show: true,   
        interval: function (index) {   
        // [false, true, true, false],
        // [false, true, false, true],
        return testArr[dataIndex][index]   
  }

axis's lastTick will last.
eg:[false, true, true, false]
image
eg:[false, true, false, true]
image

i review my submit , i think init tickLen should like this

        var dataExtent = this.scale.getExtent();
        //  obtain all the ticks
        var tickLen = this.scale.getTicks().length + dataExtent[0];
        fixOnBandTicksCoords(
            this, ticksCoords, result.tickCategoryInterval, alignWithLabel, opt.clamp,tickLen
        );
    ... 

after that, i test my case and axis-interval2.html with dataZoon, it's work now.
will pull request again, thanks you time again

Copy link
Contributor

@pissang pissang Oct 18, 2019

Choose a reason for hiding this comment

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

Great! Looks good to me now:)

@pissang
Copy link
Contributor

pissang commented Oct 15, 2019

Hi @foolzhang. I checked the code. Very neat fix! But it breaks the test case with dataZoom in axis-interval2.html.

I've left my change suggestions in the comment. Really nice job!

Copy link
Contributor Author

@foolzhang foolzhang left a comment

@pissang thank you

var tickLen = this.scale.getTicks().length;
var dataExtent = this.scale.getExtent();
var tickLen = this.scale.getTicks().length + dataExtent[0];
Copy link
Contributor Author

@foolzhang foolzhang Oct 16, 2019

Choose a reason for hiding this comment

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

major changes

@pissang pissang merged commit 4d3ae67 into apache:master Oct 18, 2019
Copy link
Contributor

@Ovilia Ovilia left a comment

There are some formatting problems. But I think it's OK to merge this first.

src/coord/Axis.js Show resolved Hide resolved
src/coord/Axis.js Show resolved Hide resolved
src/coord/Axis.js Show resolved Hide resolved
test/axis-interval.html Show resolved Hide resolved
test/axis-interval.html Show resolved Hide resolved
Ovilia added a commit that referenced this pull request Oct 18, 2019
Ovilia added a commit that referenced this pull request Oct 18, 2019
earo-Lau pushed a commit to earo-Lau/incubator-echarts that referenced this pull request Oct 21, 2019
…hart4 and echart3.8 (apache#11186)

* FIX apache#11138 DOC ERROR

* fix issues/11176

* delete console.log

* add test demo

* fix bug axis tick with dataZoom attr
earo-Lau pushed a commit to earo-Lau/incubator-echarts that referenced this pull request Oct 21, 2019
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

3 participants