Add :hover styleRule for modebar.bgcolor #3397
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
... and .ease-bg class to match modebar button opacity
transition
Contributor
Author
|
Ping @antoinerg - could you take a look at this today? I'll ❤️ to include this in 1.43.2. Thanks! |
antoinerg
reviewed
Jan 7, 2019
| 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); | ||
| Lib.addRelatedStyleRule(modeBarId, bgSelector + '#' + modeBarId, 'background-color: ' + style.bgcolor); |
Contributor
There was a problem hiding this comment.
Would this work without bgSelector? modeBarId should already be unique so this might not be necessary.
Contributor
Author
There was a problem hiding this comment.
Maybe there's a way (not a CSS expert here), but I think that :hover pseudo-class must be tagged onto the graph div class.
Contributor
There was a problem hiding this comment.
Ok thanks for the explanation. This makes sense!
antoinerg
reviewed
Jan 7, 2019
test/jasmine/tests/modebar_test.js
Outdated
| }); | ||
|
|
||
| it('changes background color (displayModeBar: true)', function(done) { | ||
| function getStyleRule() { |
Contributor
There was a problem hiding this comment.
Nonblocking but this function is the same as the one above. Could be DRYed for bonus points!
Contributor
|
Thanks for the fix and the nice tests @etpinard 💃 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
... and a
.ease-bgclass to match modebar button opacity transition.fixes #3388 - for example:
cc @plotly/plotly_js - I would appreciate if modebar-expert @antoinerg would test this one out. Thanks!