feat(broadcast): add getSelection/setSelection and selectionUpdate - #330
Open
jtj70 wants to merge 1 commit into
Open
feat(broadcast): add getSelection/setSelection and selectionUpdate#330jtj70 wants to merge 1 commit into
jtj70 wants to merge 1 commit into
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
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.
This adds the two remaining broadcast methods listed in the TODO checklist in
packages/broadcast/src/types.ts:getSelection/setSelection, plus aselectionUpdateevent so external callers can react to selection changes made inside the app (clicks, marquee, etc.), not just ones they initiated themselves.What's included
SerializedSelectionState { nodeIds: string[]; edgeIds: string[] }wire type, independent of the internalSelectionState(which can only be nodes or edges at a time).deserializeSelectiondocuments the precedence rule for the case where both arrays are non-empty:nodeIdswins, since Gephi Lite cannot represent a simultaneous nodes+edges selection today.BroadcastClientbinds to the existingselectionAtomand only emitsselectionUpdatewhen the effective selection actually changes. Atoms compare by reference, so a content-level check (selectionStatesAreEqual) was needed to avoid firing on every no-opselect()/setSelection()call, and to avoid echo-loops when a caller sets back a selection it just received.setGraphDatasetnow prunes the current selection against the new dataset (pruneSelectionToGraph), keeping ids that still exist and clearing the rest. Previously a dataset swap over broadcast could leave a stale selection referencing nodes/edges that no longer exist.setSelectionprunes unknown ids the same way, so a caller can't pollute the internal state with ids that don't exist in the current graph.ITypedEventEmitter's listener typing assumed every event was argument-less (true for the only existing event,newInstance). Generalized it via a newEventListener<F>helper so a listener can correctly receive an event's data, typed from its declared return type — needed becauseselectionUpdateis the first event to actually carry a payload.Tests
packages/gephi-lite/src/core/broadcast/client.spec.ts, 4 cases:getSelectionreturns the current selectionsetSelectionchanges the selection and emitsselectionUpdatesetGraphDatasetkeeps still-valid selected ids and drops stale onesselectionUpdatewhen the effective selection doesn't changeSeparately noticed, not part of this PR
While testing
setGraphDataset, I found a pre-existing, unrelated bug:filteredGraphAtom(core/graph/index.ts) is aderivedAtomwithoutcheckOutput: false, andlodash.isEqualtreats any two differentgraphologyGraphinstances of equal node/edge count as equal — so a same-size dataset swap can leave it silently stale, which can later makeresetCamera()throw. Reproduces without any of this PR's changes. Not fixing it here to keep this PR focused; happy to open a separate issue/PR if useful.Open questions for maintainers
Very open to a different wire shape for
SerializedSelectionStateor different dataset-replacement semantics if you'd prefer something else here — this is meant as a starting point matching the existinggetFilters/setFilterspattern, not a fixed proposal.