Skip to content

histogram: in hover labels, harmonize start/end values of shared bins - #4662

Merged
antoinerg merged 2 commits into
masterfrom
fix-4648
Mar 19, 2020
Merged

histogram: in hover labels, harmonize start/end values of shared bins#4662
antoinerg merged 2 commits into
masterfrom
fix-4648

Conversation

@antoinerg

@antoinergantoinerg commented Mar 18, 2020

Copy link
Copy Markdown
Contributor

Comment threadsrc/traces/histogram/calc.js Outdated
roundFn = function(v, isRightEdge) {
return function() {
var roundFnOpts = gd._fullLayout._roundFnOpts[groupName];
return getBinSpanLabelRound(roundFnOpts.leftGap, roundFnOpts.rightGap, binEdges, pa, calendar)(v, isRightEdge);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This line is getting little too long. Maybe:

returngetBinSpanLabelRound(roundFnOpts.leftGap,roundFnOpts.rightGap,binEdges,pa,calendar)(v,isRightEdge);

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Done in c387f9b

var roundFnOpts = {leftGap: Infinity, rightGap: Infinity};
if(groupName) {
if(!gd._fullLayout._roundFnOpts[groupName]) gd._fullLayout._roundFnOpts[groupName] = roundFnOpts;
roundFnOpts = gd._fullLayout._roundFnOpts[groupName];

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Non-blocking,
We could rewrite this:

if(!gd._fullLayout._roundFnOpts[groupName]){gd._fullLayout._roundFnOpts[groupName]=roundFnOpts;}roundFnOpts=gd._fullLayout._roundFnOpts[groupName];

to make it easier to read 3 gd._fullLayout._roundFnOpts[groupName].

@archmoj

Copy link
Copy Markdown
Contributor

This branch is little behind the master.
To help Q&A it may be a good idea to merge the master into this branch.

@archmoj

Copy link
Copy Markdown
Contributor

Screenshot from 2020-03-18 20-43-44

@antoinerg

Copy link
Copy Markdown
ContributorAuthor

This branch is little behind the master.
To help Q&A it may be a good idea to merge the master into this branch.

Is there anything that landed on master that is related to histogram?

@alexcjohnsonalexcjohnson left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

💃 LGTM!

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugsomething broken

Projects

None yet

Development

Successfully merging this pull request may close these issues.

stacked histograms have different start/end values on hover

3 participants

@antoinerg@archmoj@alexcjohnson