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

feat: add activateOn to interval1D and interval2D to define when a selection should be activated #35

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

domoritz
Copy link
Member

@domoritz domoritz commented Mar 2, 2023

I found that for very large datasets where index creation takes a few seconds, it can be better to only activate a selection on click instead of hover.

@domoritz domoritz requested a review from jheer March 2, 2023 05:18
@jheer
Copy link
Member

jheer commented Mar 2, 2023

I like this! However, we might want to wait on merging for now. I also want to think through some complementary possible changes, such as adding a query queue in the Coordinator (not just within a DB client) to enable more fine-grained control of execution / ordering / canceling. Ultimately such changes may prove orthogonal to this one, but I want to approach it holistically.

@domoritz
Copy link
Member Author

domoritz commented Mar 2, 2023

I like the idea of a more holistic approach. It would be great to also support activating a chart when the user hovers for some time rather than just on a simple mousenter or mousedown event.

I will need this change for a local experiment but we don't need to merge it yet.

@jheer
Copy link
Member

jheer commented Mar 2, 2023

Ooh, having a reusable set of activation methods (shared across various interactors) sounds like a promising direction. Would be worth enumerating what we think that space of methods should be.

@domoritz
Copy link
Member Author

domoritz commented Mar 2, 2023

I'll start

@jheer
Copy link
Member

jheer commented Mar 2, 2023

To which I would simply add:

  • Prefetch on load (i.e. precompute all indexes for individual active views/clauses)

Right now individual interactors make their own activation decisions. This conversation is making me also think of more coordinated strategies. Right now Selections include resolution strategies (which I plan to abstract to make "pluggable" in the future). Selections may also be the right place to include activation strategies, which might be shared across multiple interactors.

@domoritz
Copy link
Member Author

domoritz commented Mar 2, 2023

This reminds me of another feature. I'd like to restrict the number of brushes. It would be nice to have only one brush or two brushes as then we could make good guarantees about the performance or precompute indexes (a la immens).

@jheer
Copy link
Member

jheer commented Mar 2, 2023

Single brush support is already provided at the Selection level via the single resolution strategy. What remains is to standardize some kind of reset() method on a (to-be-formalized) "Interactor"/"SelectionSource" interface. That way a Selection can inform sources to remove visual brushes, etc., when a source's selection clause is removed. The corresponding implementation details should be easy once we decide on the interfaces/architecture.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants