Skip to content
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

VL 3.2 breaks multi mouseover selection handling? #4879

Closed
jheer opened this issue Apr 15, 2019 · 4 comments
Closed

VL 3.2 breaks multi mouseover selection handling? #4879

jheer opened this issue Apr 15, 2019 · 4 comments
Assignees
Labels
Bug 🐛 P1 Critical -- to fix ASAP

Comments

@jheer
Copy link
Member

jheer commented Apr 15, 2019

Consider this example multi-selection spec:

  • Prior to 3.2, shift-mouseover would color multiple points
  • Post 3.2, shift-mouseover only ever highlights a single point

Looking at the generated Vega, the issue appears to stem from here:

{
  "name": "sel32_toggle",
  "value": false,
  "on": [
    {
      "events": [{"source": "scope", "type": "mouseover"}],
      "update": "event.shiftKey"
    },
    {"events": [{"source": "scope", "type": "mouseout"}], "update": "false"}
  ]
},
{
  "name": "sel32_modify",
  "update": "modify(\"sel32_store\", sel32_toggle ? null : sel32_tuple, sel32_toggle ? null : true, sel32_toggle ? sel32_tuple : null)"
}

Notice that, upon mouseout events, the sel32_toggle goes to false no matter what, even if shiftKey is true. If I replace false on mouseout with event.shiftKey the example appears to behave more reasonably, but I'm not 100% sure this is the right solution, as I'm assuming this bug was introduced while trying to resolve some other issue?

In any case this is breaking the interaction notebook I wrote that is going to students later this week. I may simply have to rewrite the notebook to avoid the issue, but in any case I'm flagging this as a high priority bug so that it gets attention and we can decide what needs to be done.

@jheer jheer added Bug 🐛 P1 Critical -- to fix ASAP labels Apr 15, 2019
@jheer
Copy link
Member Author

jheer commented Apr 15, 2019

This issue also appears to have broken the documentation example here:
https://vega.github.io/vega-lite/docs/selection.html#conditional-encodings

@jheer
Copy link
Member Author

jheer commented Apr 15, 2019

Looking over the 3.2 release notes and files modified, I'm guessing the new selection clear functionality is the most likely culprit: #4720

Flagging @allenjlee!

@jheer
Copy link
Member Author

jheer commented Apr 15, 2019

Yup, found the root cause here:
https://github.com/vega/vega-lite/pull/4720/files#diff-76015606d4b117c2e2082fc707e94a07R47

It appears I can add clear: false to suppress the error, though I'd prefer not to have to do this. I'm not sure why we are using mouseout to automatically clear mouseover interactions by default.

@allenjlee
Copy link
Contributor

allenjlee commented Apr 15, 2019

A temporary solution right now is to set clear: false in the Vega-Lite specification under sel32 in your specification example. This will return to the appropriate behavior.

It seems like the problem is because we append the following to on in _toggle.

{"events": [{"source": "scope", "type": "mouseout"}], "update": "false"}

However, this is needed for other examples, specifically: https://vega.github.io/vega-lite/examples/interactive_query_widgets.html. I'm not sure what the best way to resolve this bug is, one way is to flag it so that it doesn't automatically add mouseout upon seeing mouseover for _toggle. What do you think @arvind?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug 🐛 P1 Critical -- to fix ASAP
Projects
None yet
Development

No branches or pull requests

4 participants