Skip to content

Fix Shape tool type dropdown not persisting selection and not excluding Line/Rectangle/Ellipse#3731

Open
Ayush2k02 wants to merge 4 commits intoGraphiteEditor:masterfrom
Ayush2k02:preserve-shape-options
Open

Fix Shape tool type dropdown not persisting selection and not excluding Line/Rectangle/Ellipse#3731
Ayush2k02 wants to merge 4 commits intoGraphiteEditor:masterfrom
Ayush2k02:preserve-shape-options

Conversation

@Ayush2k02
Copy link
Contributor

@Ayush2k02 Ayush2k02 commented Feb 7, 2026

This regression was introduced in #2914 (commit 523132d)

In the SetShape handler, this line was added:
responses.add(ShapeToolMessage::UpdateOptions(ShapeOptionsUpdate:

since the setShape was being called with polygon always , the added UpdateOptions call overwrites options.shape_type

below is the call
responses.add(ShapeToolMessage::SetShape { shape: ShapeType::Polygon });

Below are the two main fixes from this PR

  • Remove UpdateOptions from SetShape handler - Now SetShape only updates current_shape and calls update_dynamic_hints directly for hint updates, without touching options.shape_type.
  • Add SyncShapeWithOptions message - When activating the Shape tool, sync current_shape to match options.shape_type. This ensures the drawing state matches the dropdown selection, especially when returning from Line/Rectangle/Ellipse alias tools.
Screen.Recording.2026-02-16.at.6.59.52.PM.mov

Fixes the issue from this discussion
https://discord.com/channels/731730685944922173/731738914812854303/1469590633361707204

@Ayush2k02
Copy link
Contributor Author

Screenshot 2026-02-09 at 10 09 56 AM

@Keavon Keavon force-pushed the preserve-shape-options branch from b43b8cc to 7b14fce Compare February 12, 2026 23:37
@Keavon
Copy link
Member

Keavon commented Feb 12, 2026

Thanks for fixing these two bugs. But I have a concern, which is that your implementation involved new logic, when in both cases, this already used to work and they each broke as a regression at some point in the past few months. I would expect an implementation should track down what broke and then fix what existing logic already performed the functionality for these two issues, rather than leaving behind the other logic in some unknown state and building new logic. Could you please spend some time to go back and check which commit broke both of these issues and post those PRs or commit hashes in a comment here, and then investigate what specific changes within each PR resulted in the regression, and then checking what minimal changes are required to fix the issues and seeing if the ones you did here are redundant?

@Keavon Keavon marked this pull request as draft February 12, 2026 23:47
@Keavon Keavon changed the title persist shape tool dropdown selection Fix Shape tool type dropdown not persisting selection and not excluding Line/Rectangle/Ellipse Feb 12, 2026
@Ayush2k02
Copy link
Contributor Author

Yeah, makes sense to fix that. Let me do that

@Keavon
Copy link
Member

Keavon commented Feb 15, 2026

Any updates? Just checking how it's going, since this fix will be one of the more important ones to have merged.

@Ayush2k02
Copy link
Contributor Author

Ayush2k02 commented Feb 16, 2026 via email

@Ayush2k02
Copy link
Contributor Author

#2914

Its this one, from where the bug originated. Can share the bisect logs if you want. I'll fix this now

@Ayush2k02 Ayush2k02 force-pushed the preserve-shape-options branch from 7d0ba68 to c704d0a Compare February 16, 2026 12:25
@Ayush2k02
Copy link
Contributor Author

Have updated the video and PR description, this is reviewable from my side now

@Ayush2k02 Ayush2k02 marked this pull request as ready for review February 16, 2026 13:42
@Keavon
Copy link
Member

Keavon commented Feb 16, 2026

!build

@github-actions
Copy link

📦 Build Complete for fa7f9b5

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.

2 participants