Addressing #4328 #4355
Open
thermalPasteMilkshake wants to merge 3 commits into
Open
Conversation
Contributor
There was a problem hiding this comment.
1 issue found across 1 file
Confidence score: 2/5
- In
editor/src/messages/tool/tool_messages/select_tool.rs, the Deepest-selection edit handler is replaced with an empty block, which appears to disable double-click-to-edit for text/path layers and leavesedit_layer_deepest_manipulationeffectively unused; merging as-is likely causes a clear editing workflow regression in the default selection mode — restore or rewire the Deepest edit path (and verify with a focused interaction test) before merging.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="editor/src/messages/tool/tool_messages/select_tool.rs">
<violation number="1" location="editor/src/messages/tool/tool_messages/select_tool.rs:1129">
P1: Replacing the Deepest-selection edit handler with an empty block silently disables double-click-to-edit for text/path layers in the default 'Deepest' selection mode, and leaves `edit_layer_deepest_manipulation` as dead code. This looks like leftover debugging rather than an intentional fix and should be reverted or reworked.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| match tool_data.nested_selection_behavior { | ||
| NestedSelectionBehavior::Shallowest => edit_layer_shallowest_manipulation(document, intersect, responses), | ||
| NestedSelectionBehavior::Deepest => edit_layer_deepest_manipulation(intersect, &document.network_interface, responses), | ||
| NestedSelectionBehavior::Deepest => {} |
Contributor
There was a problem hiding this comment.
P1: Replacing the Deepest-selection edit handler with an empty block silently disables double-click-to-edit for text/path layers in the default 'Deepest' selection mode, and leaves edit_layer_deepest_manipulation as dead code. This looks like leftover debugging rather than an intentional fix and should be reverted or reworked.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At editor/src/messages/tool/tool_messages/select_tool.rs, line 1129:
<comment>Replacing the Deepest-selection edit handler with an empty block silently disables double-click-to-edit for text/path layers in the default 'Deepest' selection mode, and leaves `edit_layer_deepest_manipulation` as dead code. This looks like leftover debugging rather than an intentional fix and should be reverted or reworked.</comment>
<file context>
@@ -1125,7 +1126,7 @@ impl Fsm for SelectToolFsmState {
match tool_data.nested_selection_behavior {
NestedSelectionBehavior::Shallowest => edit_layer_shallowest_manipulation(document, intersect, responses),
- NestedSelectionBehavior::Deepest => edit_layer_deepest_manipulation(intersect, &document.network_interface, responses),
+ NestedSelectionBehavior::Deepest => {}
}
}
</file context>
Suggested change
| NestedSelectionBehavior::Deepest => {} | |
| NestedSelectionBehavior::Deepest => edit_layer_deepest_manipulation(intersect, &document.network_interface, responses), |
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.
Hello, This fixes #4328
(this is just a rough fix, I just wanted to ask some stuff out alongside it)
Im also requesting guidance on the outer border removal as I stated in the discussions for the issue.
#4328 (comment)
In the discussions its also mentioned that the text tool stays selected even when the editing is done. To that I also have few questions.
in the part of the code determining the edits, all except one
TextToolFsmState::Editingleads to another state. Which does make sense coz we don't really know when user is "done" editing.The most easy solution seems to be when user is simply clicking outside text border.
This is roughly how I assume we can do that:
(TextToolFsmState::Editing && UserClicksOutsideBorder) => { TextToolFsmState::Ready; }With this context I wanted to ask if there is any API to check if user has clicked outside border?