-
Notifications
You must be signed in to change notification settings - Fork 621
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: for interactive charts, changed cursor to pointer (also updated tests) #9305
Conversation
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.
I don't think this is quite right yet. For example in http://localhost:5173/editor/#/examples/vega-lite/dynamic_color_legend we show a pointer cursor but the selection is an interval selection with a brush. For brushes, the cursor should change for the whole chart (where you can draw the brush) and not change just because you hover over a mark. Think of it this way: an interval selection isn't a selection of a mark so the cursor should not be specific to the mark.
…point selection (rather than interval) and no binding
|
||
let isPoint = false; | ||
let isBind = false; | ||
if (model.component.selection) { |
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.
this doesn't look right. What if there is one point selection and one point selection with bind. Then this would not work.
} | ||
} | ||
} | ||
interactive && isPoint && !isBind ? (model.markDef.cursor ??= 'pointer') : {}; |
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.
why not use an if statement?
Replaced by #9358 |
PR Description
Fixes #4155 (specifically for cursor over interactive bars)