Skip to content

Add G/S shortcut handling to Artboard tool#4039

Draft
devviktoria wants to merge 1 commit intoGraphiteEditor:masterfrom
devviktoria:fix-4026-artboard-shortcuts
Draft

Add G/S shortcut handling to Artboard tool#4039
devviktoria wants to merge 1 commit intoGraphiteEditor:masterfrom
devviktoria:fix-4026-artboard-shortcuts

Conversation

@devviktoria
Copy link
Copy Markdown

Adds support G/S (grab and scale) keyboard shortcut handling to the Artboard tool.

Closes #4026

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request implements keyboard shortcuts for grabbing and scaling artboards within the Artboard tool by adding a new GS message and updating the FSM logic. Review feedback highlights that the scaling implementation is incomplete, lacking necessary state initializations and document transactions required for the ResizingBounds state to function correctly. Additionally, it is recommended to remove commented-out code in the message handler.

Comment on lines +310 to +313
} else if input.keyboard.key(scale) && tool_data.selected_artboard.is_some() {
//tool_data.start_resizing(selected_edges, document, input);
tool_data.get_snap_candidates(document, input);
ArtboardToolFsmState::ResizingBounds
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

The scale branch in the GS message handler is missing several critical initializations required for the ResizingBounds state to function correctly. Specifically, it needs to:

  1. Start a document transaction so the operation can be undone/aborted.
  2. Initialize bounding_box_manager.selected_edges and opposite_pivot. Without selected_edges, the resize_artboard function (called during PointerMove) will return early and do nothing.
  3. Call tool_data.start_resizing to set up the center of transformation and initial location.

Since this is a keyboard shortcut, you might want to default to resizing from the bottom-right corner or similar.


ArtboardToolFsmState::Dragging
} else if input.keyboard.key(scale) && tool_data.selected_artboard.is_some() {
//tool_data.start_resizing(selected_edges, document, input);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Please remove the commented-out code. If the functionality is intended to be part of this PR, it should be implemented; otherwise, it should be removed to keep the codebase clean.

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.

The Artboard tool needs to support G/S (grab and scale) transformation

1 participant