Skip to content

Improve the Path tool's segment editing mode #2860

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

Draft
wants to merge 11 commits into
base: master
Choose a base branch
from

Conversation

4adex
Copy link
Collaborator

@4adex 4adex commented Jul 11, 2025

Closes #2832

@Keavon
Copy link
Member

Keavon commented Jul 13, 2025

  • Two more issues in this video. First part shows being able to hover a point even if it's invisible. Second part shows how the new hover styling (which I updated the visuals of) turns white when it's already selected, but should only apply the visual change to the outside while keeping the inner part solid blue because it's still selected.

    capture_34_.mp4
    • We also want the anchors/handles to light up, as if hovered, when currently within the box/lasso selection. And segments while in segment mode.
  • Shift clicking the point/segment mode checkboxes still lets you deselect all of them, but deselecting all shouldn't be possible.

  • If I have a segment selected in segment mode, switching to point mode keeps that segment selected which shouldn't be possible.

  • Upon inserting a point, it needs to not select the point, while in segment mode, since point selection shouldn't ever occur in that mode.

@4adex
Copy link
Collaborator Author

4adex commented Jul 27, 2025

#2860 (comment)
All of these are also covered in this PR, double checked.

@Keavon
Copy link
Member

Keavon commented Jul 27, 2025

Can you please explain the idea behind the frozen flag in the CheckboxInput, why that's necessary (what consideration was taken for doing it purely in the backend?), etc.?

@4adex
Copy link
Collaborator Author

4adex commented Jul 27, 2025

I first did it purely in backend only, so these were the two approaches that I tried -

  • I used the disabled flag (disabled when only one mode is activated) so that you cannot turn both modes to false, as in https://130b5a88.graphite.pages.dev/ but then it also greys out the button and it doesn't look good for the UX that which mode is active and what can be actually clicked.
  • Then I tried without the disabled flag, in which the options don't update if only that specific mode is enabled. But you can still click on the button and for a very short moment you will see the button to be off and then it turns back to white in the next frame, which worked but also was bad UX.

Hence I decided to make a flag frozen similar to disabled, which works as disabled button but with the primary color (without being greyed out).

@Keavon Keavon changed the title Improve path editing mode Improve the Path tool's segment editing mode Jul 27, 2025
@Keavon
Copy link
Member

Keavon commented Jul 27, 2025

Can you go back to the approach from your second bullet point, where it briefly flashes as disabled? We can live with that for the moment. What I need to do is adapt the UI design system to better accommodate the similarities ranging from checkboxes to radio buttons so they can share certain parts of their behavior, as needed here. The little flaw can be fixed at that time.

@Keavon
Copy link
Member

Keavon commented Jul 27, 2025

  • Please ensure that, while in segment editing mode, I can still drag handles.
  • In both segment and point mode, when I'm clicking down to select an anchor/handle or segment, we need the gray ghost-of-the-original-shape outline to not show up until dragging has begun, since this flashes when the mouse is down. Most visible on an open path like a "C" shape which still has a Fill node causing the straight line between endpoints to show this. (I realize this is somewhat unrelated but still worth mentioning here.)
  • After Ctrl-clicking to insert a point on a selected segment, the resulting two segments should also be selected. This overrides the bullet point I'd written above:

    Upon inserting a point, it needs to not select the point, while in segment mode, since point selection shouldn't ever occur in that mode.

@@ -1645,14 +1764,72 @@ impl Fsm for PathToolFsmState {
};

let quad = tool_data.selection_quad(document.metadata());
let polygon = &tool_data.lasso_polygon;

// TODO: Calculate which points/ handles are currently in the selected region and make those have a
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Incomplete comment sentence

@Keavon
Copy link
Member

Keavon commented Jul 27, 2025

I pushed my code review tweaks. I'll mark this as a draft while waiting the other requests in today's comments above. We're close on this one now!

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

Successfully merging this pull request may close these issues.

Path tool segment editing mode polish
2 participants