fix: scope edge movement to current graph - #551
Open
caydyan wants to merge 1 commit into
Open
Conversation
1 task
Author
|
Current-head validation rerun on
Local worktree is clean after the rerun. |
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.
Summary
Fixes #513.
Edges were being moved with
document.querySelector('.svelvet-graph-wrapper'), which always returns the first Svelvet graph wrapper on the page. In pages with multiple Svelvet instances, that caused edges from later instances to be appended into the first graph.This changes edge movement to use
edgeElement.closest('.svelvet-graph-wrapper'), so each edge is appended to the graph wrapper it was rendered inside. It also avoids removing the edge when it is already parented by the correct wrapper.Test coverage
Added a jsdom unit test that builds two
.svelvet-graph-wrappercontainers and verifies an edge from the second wrapper stays under that second wrapper instead of moving to the first one.Validation
npm run test:unit -- tests/unit-tests/components/edgeMove.test.tsnpm run test:unitnpx prettier --check src/lib/components/Edge/Edge.svelte tests/unit-tests/components/edgeMove.test.tsnpx eslint src/lib/components/Edge/Edge.svelte tests/unit-tests/components/edgeMove.test.tsnpm run buildKnown existing failure:
npm run checkstill reports the existing repo-wide 64 errors / 5 warnings in unrelated files such asDrawerController.svelte,createGraph.ts, route examples, and existing tests.Sphinx bounty #2284 payout address:
bc1qev5ant33v5y89qqjvcf4mh9hlax5svqf5xd7gc.