Skip to content

Add Rectangle tool controls for transforming existing rectangles #2315

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

Closed
wants to merge 5 commits into from

Conversation

Nitish-bot
Copy link
Contributor

Closes part of #1715

@Keavon
Copy link
Member

Keavon commented Feb 26, 2025

!build

Copy link

📦 Build Complete for ec00b5b
https://2726e16d.graphite.pages.dev

@0HyperCube 0HyperCube reopened this Mar 6, 2025
@0HyperCube
Copy link
Member

@Nitish-bot I fixed the transform issue by reusing the code from the text tool PR #2181.

Copy link

github-actions bot commented Mar 6, 2025

📦 Build Complete for 46cab9a
https://6fc8ec54.graphite.pages.dev

@0HyperCube
Copy link
Member

@Keavon is there anything else that needs to be added to this PR?

@Keavon
Copy link
Member

Keavon commented Mar 9, 2025

@0HyperCube are you concerned about the +250 lines this adds? That feels excessive, but you'd know better than me.

There are a couple of concerns that we could save for a followup PR:

  • Corner rotation doesn't work, despite the cursor icon showing it
  • With both this and the Line tool, we don't want to show the handles during the initial drawing, only after the drawing is done
  • Line tool still needs abort, but it looks like Rectangle correctly has it which I'm glad to see

I'll wait to hear your thoughts about the potentially excessive line count before proceeding to merge this if that's ok.

@Keavon Keavon marked this pull request as ready for review March 9, 2025 21:17
@Keavon Keavon changed the title Rectangle tool allows modifying rectangle Add Rectangle tool controls for transforming existing rectangles Mar 9, 2025
@0HyperCube
Copy link
Member

There is some code duplication with the text tool @Keavon. However the transform bounds/cage system is quite verbose to work with and probably needs to be simplified and refactored, which is the main cause of the complexity.

I don't really think we should add the whole transform bounds/cage to the rectangle tool as we are just duplicating the functionality of the select tool. I would much prefer to do something like Inkscape where only a couple of handles are displayed that allow you to visually set the values in the node. For example the star tool looks like this:
star tool in inkscape showing 2 handles

@Keavon
Copy link
Member

Keavon commented Mar 14, 2025

@0HyperCube Yep, good call, that's the plan. @mTvare6 is currently working to refactor all the shape drawing tools so they have a single combined implementation with control over which shape is used: polygon and star (plus more primitive geometric shapes in the near future) will be part of the Polygon tool (likely to be renamed the Shape tool) and the most common ones (Line, Rectangle, and Ellipse) will have a dedicated tool icon but will just be a choice of shape like star/polygon/etc. And each individual primitive shape will be in charge of defining its own gizmos, for example similar to the star you posted the screenshot of above. The Rectangle will specify handles for corners and edges which would appear similar to the Transform cage of the Select tool, but is actually just its own gizmo controls similar to the star image above but for rectangles.

@0HyperCube 0HyperCube closed this Mar 14, 2025
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.

3 participants