-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
modeBar styling #3068
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
modeBar styling #3068
Changes from all commits
1b7c126
947eaf2
212612b
3795223
e870e34
a4d3948
61d1c7b
1d4ac7e
ff9d801
397a6de
baeb1bc
254f158
f9e8777
fc47c72
0046f12
b7c2355
31d6fb2
97f25e2
dc7877b
f5cc0e9
452f9bc
266d63b
ff3f9f6
db9edd1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -14,7 +14,7 @@ var isNumeric = require('fast-isnumeric'); | |
|
||
var Lib = require('../../lib'); | ||
var Icons = require('../../../build/ploticon'); | ||
|
||
var Parser = new DOMParser(); | ||
|
||
/** | ||
* UI controller for interactive plots | ||
|
@@ -45,13 +45,29 @@ var proto = ModeBar.prototype; | |
proto.update = function(graphInfo, buttons) { | ||
this.graphInfo = graphInfo; | ||
|
||
var context = this.graphInfo._context; | ||
var context = this.graphInfo._context, | ||
fullLayout = this.graphInfo._fullLayout, | ||
modeBarId = 'modebar-' + fullLayout._uid; | ||
|
||
this.element.setAttribute('id', modeBarId); | ||
this._uid = modeBarId; | ||
|
||
if(context.displayModeBar === 'hover') { | ||
this.element.className = 'modebar modebar--hover'; | ||
} | ||
else this.element.className = 'modebar'; | ||
|
||
if(fullLayout.modebar.orientation === 'v') { | ||
this.element.className += ' vertical'; | ||
buttons = buttons.reverse(); | ||
} | ||
|
||
Lib.deleteRelatedStyleRule(modeBarId); | ||
Lib.addRelatedStyleRule(modeBarId, '#' + modeBarId, 'background-color: ' + fullLayout.modebar.bgcolor); | ||
Lib.addRelatedStyleRule(modeBarId, '#' + modeBarId + ' .modebar-btn .icon path', 'fill: ' + fullLayout.modebar.color); | ||
Lib.addRelatedStyleRule(modeBarId, '#' + modeBarId + ' .modebar-btn:hover .icon path', 'fill: ' + fullLayout.modebar.activecolor); | ||
Lib.addRelatedStyleRule(modeBarId, '#' + modeBarId + ' .modebar-btn.active .icon path', 'fill: ' + fullLayout.modebar.activecolor); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These don't appear to update properly on There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let me investigate As for the screen capture: the gray icons are the ones that are currently active (gray being the default There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Ha I see. My mistake! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Commit 97f25e2 fixes the issue with The new style rules were just appended to the same There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Cool. Let's 🔒 this down in a jasmine test though. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In commit f5cc0e9, we test each new layout attributes and make sure they behave properly when using There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Great tests. Thanks 🎉 |
||
|
||
// if buttons or logo have changed, redraw modebar interior | ||
var needsNewButtons = !this.hasButtons(buttons); | ||
var needsNewLogo = (this.hasLogo !== context.displaylogo); | ||
|
@@ -65,7 +81,12 @@ proto.update = function(graphInfo, buttons) { | |
this.updateButtons(buttons); | ||
|
||
if(context.displaylogo) { | ||
this.element.appendChild(this.getLogo()); | ||
if(fullLayout.modebar.orientation === 'v') { | ||
this.element.prepend(this.getLogo()); | ||
} else { | ||
this.element.appendChild(this.getLogo()); | ||
} | ||
|
||
this.hasLogo = true; | ||
} | ||
} | ||
|
@@ -173,31 +194,42 @@ proto.createButton = function(config) { | |
* @Param {object} thisIcon | ||
* @Param {number} thisIcon.width | ||
* @Param {string} thisIcon.path | ||
* @Param {string} thisIcon.color | ||
* @Return {HTMLelement} | ||
*/ | ||
proto.createIcon = function(thisIcon) { | ||
var iconHeight = isNumeric(thisIcon.height) ? | ||
Number(thisIcon.height) : | ||
thisIcon.ascent - thisIcon.descent, | ||
svgNS = 'http://www.w3.org/2000/svg', | ||
icon = document.createElementNS(svgNS, 'svg'), | ||
path = document.createElementNS(svgNS, 'path'); | ||
icon; | ||
|
||
icon.setAttribute('height', '1em'); | ||
icon.setAttribute('width', (thisIcon.width / iconHeight) + 'em'); | ||
icon.setAttribute('viewBox', [0, 0, thisIcon.width, iconHeight].join(' ')); | ||
if(thisIcon.path) { | ||
icon = document.createElementNS(svgNS, 'svg'); | ||
icon.setAttribute('viewBox', [0, 0, thisIcon.width, iconHeight].join(' ')); | ||
icon.setAttribute('class', 'icon'); | ||
|
||
var path = document.createElementNS(svgNS, 'path'); | ||
path.setAttribute('d', thisIcon.path); | ||
|
||
path.setAttribute('d', thisIcon.path); | ||
if(thisIcon.transform) { | ||
path.setAttribute('transform', thisIcon.transform); | ||
} | ||
else if(thisIcon.ascent !== undefined) { | ||
// Legacy icon transform calculation | ||
path.setAttribute('transform', 'matrix(1 0 0 -1 0 ' + thisIcon.ascent + ')'); | ||
} | ||
|
||
if(thisIcon.transform) { | ||
path.setAttribute('transform', thisIcon.transform); | ||
icon.appendChild(path); | ||
} | ||
else if(thisIcon.ascent !== undefined) { | ||
// Legacy icon transform calculation | ||
path.setAttribute('transform', 'matrix(1 0 0 -1 0 ' + thisIcon.ascent + ')'); | ||
|
||
if(thisIcon.svg) { | ||
var svgDoc = Parser.parseFromString(thisIcon.svg, 'application/xml'); | ||
icon = svgDoc.childNodes[0]; | ||
} | ||
|
||
icon.appendChild(path); | ||
icon.setAttribute('height', '1em'); | ||
icon.setAttribute('width', '1em'); | ||
|
||
return icon; | ||
}; | ||
|
@@ -272,7 +304,7 @@ proto.getLogo = function() { | |
a.setAttribute('data-title', Lib._(this.graphInfo, 'Produced with Plotly')); | ||
a.className = 'modebar-btn plotlyjsicon modebar-btn--logo'; | ||
|
||
a.appendChild(this.createIcon(Icons.plotlylogo)); | ||
a.appendChild(this.createIcon(Icons.newplotlylogo)); | ||
|
||
group.appendChild(a); | ||
return group; | ||
|
@@ -288,6 +320,7 @@ proto.removeAllButtons = function() { | |
|
||
proto.destroy = function() { | ||
Lib.removeElement(this.container.querySelector('.modebar')); | ||
Lib.deleteRelatedStyleRule(this._uid); | ||
}; | ||
|
||
function createModeBar(gd, buttons) { | ||
|
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.
Do we have to use CSS here? Could we instead add
mouseover
/mouseout
handlers to the modebar buttons, similar to how we handle updatemenu buttons:plotly.js/src/components/updatemenus/draw.js
Lines 219 to 221 in 30ed4a4
plotly.js/src/components/updatemenus/draw.js
Lines 464 to 467 in 30ed4a4
that way we wouldn't have to hijack the
<style>
dynamically.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 could use
mouseover
/mouseout
for hover but I wasn't sure how to handle the active state. Since we already add/remove class nameactive
to the icons, I thought it made sense to just style those in CSS. @alexcjohnson wasn't against the idea but we can consider another strategy.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.
Right, getting "active" to work would require some work. The modebar button click handlers would need to update their style attribute instead of appending/removing the
"active"
class name.Consider my comment non-blocking.