-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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 hover bug #7338
Fix hover bug #7338
Conversation
@martian111 @Lexachoc Do you mind taking a look at this fix? |
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.
💃 Nice job tracking that down - and @Lexachoc thanks for the very detailed bug report!
@marthacryan Thank you for the quick PR! The original issue is now fixed.🎉 BTW, for a usecase for users using dynamic colors (CSS variables) as I noticed in #7335 (comment). The I think most people use static values, so this issue may not be suitable for this PR. Maybe I'll make a separate PR for it. The original feature request for supporting CSS variables is in here: #4915 |
@Lexachoc So glad that it worked for you! Do you mind making a separate issue for the dynamic colors? It'll help with tracking |
@marthacryan Sure. So far, I've managed to finish the dynamic colors using CSS variables for chart types: CSS variable for Not so much types so far, I'll try to cover as many types as possible, starting with the basic plots. (I haven't checked for other chart types yet. I hope it will work for most of them, too.) It's more complex than I initially thought because some plot types have a different color usage logic, e.g. Below is the effect (preview) for mock json file: {
"data": [
{
"a": [
40,
36,
51,
51,
38,
49,
31,
81,
40,
30,
77,
31,
106,
84,
55,
90,
55
],
"c": [
55,
58,
63,
77,
64,
45,
29,
52,
40,
19,
72,
33,
61,
58,
60,
51,
30
],
"b": [
46,
39,
22,
60,
37,
42,
19,
33,
36,
13,
38,
24,
50,
31,
25,
26,
21
],
"text": [
1998,
1999,
2000,
2001,
2002,
2003,
2004,
2005,
2006,
2007,
2008,
2009,
2010,
2011,
2013,
2014,
2015
],
"mode": "markers",
"type": "scatterternary"
}
],
"layout": {
"uirevision": "true",
"paper_bgcolor": "hsl(var(--background))",
"plot_bgcolor": "hsl(var(--background))",
"font": {
"family": "Geist Variable",
"size": 14,
"color": "hsl(var(--foreground))"
},
"xaxis": {
"gridcolor": "hsl(var(--border))",
"linecolor": "hsla(var(--foreground) .5)",
"zerolinecolor": "hsla(var(--border) .5)"
},
"yaxis": {
"gridcolor": "hsl(var(--border))",
"linecolor": "hsla(var(--foreground) .5)",
"zerolinecolor": "hsla(var(--border) .5)"
},
"ternary": {
"sum": 100,
"bgcolor": "hsl(var(--background))",
"aaxis": {
"gridcolor": "hsl(var(--border))",
"linecolor": "hsla(var(--foreground) .5)",
"tickcolor": "hsla(var(--foreground) .5)"
},
"baxis": {
"gridcolor": "hsl(var(--border))",
"linecolor": "hsla(var(--foreground) .5)",
"tickcolor": "hsla(var(--foreground) .5)"
},
"caxis": {
"gridcolor": "hsl(var(--border))",
"linecolor": "hsla(var(--foreground) .5)",
"tickcolor": "hsla(var(--foreground) .5)"
}
},
"colorway": [
"hsl(var(--trace-1))",
"hsl(var(--trace-2))",
"hsl(var(--trace-3))",
"hsl(var(--trace-4))",
"hsl(var(--trace-5))",
"hsl(var(--trace-6))",
"hsl(var(--trace-7))",
"hsl(var(--trace-8))",
"hsl(var(--trace-9))",
"hsl(var(--trace-10))",
"hsl(var(--trace-11))",
"hsl(var(--trace-12))"
],
"hoverlabel": {
"align": "left",
"bgcolor": "hsl(var(--primary))",
"namelength": -1,
"font": {
"family": "Geist Variable",
"size": 14,
"color": "hsl(var(--primary-foreground))"
}
},
"modebar": {
"bgcolor": "hsl(var(--background))",
"color": "hsla(var(--ring))",
"activecolor": "hsl(var(--primary))"
},
"legend": {
"font": {
"family": "Geist Variable",
"size": 14
},
"opacity": 1
},
"autosize": true
}
} |
Fixes #7335. The issue was happening because the
setStyleOnHover
function was:document.querySelectorAll('#' + modeBarId + ' .modebar-btn')
was returning an empty list, because while the elements were created, they weren't part of thedocument
.To solve #1, I moved the call to
setStyleOnHover
to after the button elements were created, and to solve #2, I passed theModeBar.element
as an optional parameter tosetStyleOnHover
. When theelement
parameter is present,querySelectorAll
is called onelement
rather thandocument
, so that it doesn't need to be attached to the DOM yet.