Fix artboard deletion #1651
Merged
Fix artboard deletion #1651
Conversation
This file contains 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
|
!build |
|
|
Nice work on the core functionality! Just one bug I notice if I draw several artboards, selecting one with the Artboard tool and deleting it causes the others to disappear (from getting disconnected in the graph). I think this is still the case in master so you didn't cause a regression, but since this is related to that bug I'd appreciate it if you'd help investigate and fix that here too. (Or feel free to say no, it can also be a followup PR.) capture.mp4 |
Keavon
force-pushed
the
fix-clear-artboards
branch
2 times, most recently
from
258cec6
to
7a9c5bd
Compare
|
!build |
|
Keavon
reviewed
Mar 8, 2024
editor/src/messages/portfolio/document/node_graph/graph_operation_message_handler.rs
Outdated
Show resolved
Hide resolved
Keavon
requested changes
Mar 9, 2024
There was a problem hiding this comment.
- The code you've added in
graph_operation_message_handler.rsis pretty complicated to read, that is, how it works isn't obvious from reading it without having to analyze it pretty closely. Could you add some comments to each line/block of lines explaining what each line/block does? Something to summarize their purposes. Thanks. - Document > Clear Artboards crashes. To reproduce: create a new document with the default artboard; use the Artboard tool to draw a new artboard; click Clear Artboards and you'll see it crashes.
|
I'm marking this as draft for the moment, please mark it as ready for review again when you're ready, and also ping me so I notice it's ready again. Thanks. |
skoriop
force-pushed
the
fix-clear-artboards
branch
from
6377048
to
57b235e
Compare
skoriop
force-pushed
the
fix-clear-artboards
branch
from
57b235e
to
b3bc4c9
Compare
Keavon
approved these changes
Mar 16, 2024
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 PR ensures that:
Document > Clear Artboardsdoes not delete its childrenArtboard Tool > Deletedoes not delete its childrenCloses #1505.