-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
histogram: remove gap when barmode
is relative
#3652
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
Conversation
src/traces/bar/layout_defaults.js
Outdated
@@ -46,7 +48,6 @@ module.exports = function(layoutIn, layoutOut, fullData) { | |||
|
|||
if(!hasBars) return; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we should
delete layoutOut.barmode;
when hasBars
is false, so that it doesn't pollute fullLayout
.
@@ -46,7 +46,10 @@ module.exports = function(layoutIn, layoutOut, fullData) { | |||
} | |||
} | |||
|
|||
if(!hasBars) return; | |||
if(!hasBars) { | |||
delete layoutOut.barmode; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
++
a jasmine test in bar_test.js
plz 😏
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
a jasmine test that checks _fullLayout
doesn't have a barmode
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, that and the converse. Similar to
plotly.js/test/jasmine/tests/bar_test.js
Lines 214 to 228 in ef932cf
it('should not include alignementgroup/offsetgroup when barmode is not *group*', function() { | |
var gd = { | |
data: [{type: 'bar', y: [1], alignmentgroup: 'a', offsetgroup: '1'}], | |
layout: {barmode: 'group'} | |
}; | |
supplyAllDefaults(gd); | |
expect(gd._fullData[0].alignmentgroup).toBe('a', 'alignementgroup'); | |
expect(gd._fullData[0].offsetgroup).toBe('1', 'offsetgroup'); | |
gd.layout.barmode = 'stack'; | |
supplyAllDefaults(gd); | |
expect(gd._fullData[0].alignmentgroup).toBe(undefined, 'alignementgroup'); | |
expect(gd._fullData[0].offsetgroup).toBe(undefined, 'offsetgroup'); | |
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done in bc3890b
That's exactly what I had in mind, thx! |
Ok, let's start merging things for 1.46.0 💃 💃 💃 |
Thanks guys! |
Fixes #3649 but I am not sure this is what @alexcjohnson had in mind!
cc @nicolaskruchten