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(time): bar bandWidth with time axis #11145 #11479

Merged
merged 5 commits into from
Oct 28, 2019
Merged

fix(time): bar bandWidth with time axis #11145 #11479

merged 5 commits into from
Oct 28, 2019

Conversation

Ovilia
Copy link
Contributor

@Ovilia Ovilia commented Oct 24, 2019

fix(time): bar bandWidth with time axis #11145

before fixing (see bar2.html):
image

after fixing:
image

Bandwidth is calculated using the min gap between two adjacent time values of all series.

There's a bug with data filtering as reported in #11478, which is not raised in this pull request and can be reproduced with v4.4.0.

src/layout/barGrid.js Outdated Show resolved Hide resolved
src/layout/barGrid.js Show resolved Hide resolved
src/layout/barGrid.js Outdated Show resolved Hide resolved
// Ignore 0 delta because they are of the same axis value
var delta = axisValues[i][j] - axisValues[i][j - 1];
delta = delta === 0 ? Number.MAX_VALUE : delta;
min = Math.min(min, delta);
Copy link
Contributor

Choose a reason for hiding this comment

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

I suggested changing these two lines to

if (delta > 0) {
  min = Math.min(min, delta)
}

to make it more clear and effecient.

@pissang
Copy link
Contributor

pissang commented Oct 28, 2019

LGTM now

@Ovilia Ovilia merged commit 794bad3 into master Oct 28, 2019
@Ovilia Ovilia deleted the fix-11145 branch October 28, 2019 12:16
src/layout/barGrid.js Show resolved Hide resolved
src/layout/barGrid.js Show resolved Hide resolved
src/layout/barGrid.js Show resolved Hide resolved
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