Skip to content
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

Add max width/height to text layers and draggable text boxes to the Text tool #2118

Merged
merged 36 commits into from
Jan 1, 2025

Conversation

Nitish-bot
Copy link
Contributor

Part of #1105

By the end, you should be able to drag with the text tool to create a bounding box and your line width should be set automatically which was hard coded to none before. Your cursor should also change to reflect change in functionality when dragging.

@Nitish-bot Nitish-bot marked this pull request as ready for review December 2, 2024 20:05
Copy link

📦 Build Complete for b14db2d
https://5aab0935.graphite.pages.dev

Copy link
Member

@0HyperCube 0HyperCube left a comment

Choose a reason for hiding this comment

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

The code looks good and it works well. Thanks for this contribution.

@0HyperCube 0HyperCube enabled auto-merge (squash) December 12, 2024 21:22
@Keavon Keavon disabled auto-merge December 12, 2024 21:33
@Keavon
Copy link
Member

Keavon commented Dec 12, 2024

I'll do a quick review before this is merged.

@0HyperCube 0HyperCube requested a review from Keavon December 16, 2024 20:02
Copy link
Member

@Keavon Keavon left a comment

Choose a reason for hiding this comment

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

QA feedback:

  • Text tool dragging to create a text area:
    • Needs support for right-clicking or Escape to abort
    • Needs support for holding the Shift modifier to make it draw a constrained square area (try this in the Rectangle tool)
    • Needs support for holding the Alt modifier to make it draw centered around the starting point of the drag (try this in the Rectangle tool)
    • Needs to work with auto-panning correctly, so the drag starting point doesn't drift
  • When the Select tool is active and the text layer is selected, it should display a second rectangle around the text (in addition to the existing interactive transform cage's rectangle) styled with a dashed line, indicating the bounds of the text area so it's clear to the user that the transform cage is affecting the text transform but there is also a separate text area that's not being dragged (until we eventually add some kind of way to let the user choose between dragging the transform vs. the text area width)
  • Before we can merge this, we need to write an upgrade script to support the new format which runs when opening a document file. Please see here for the existing code you can reference, which I wrote when I added line height and character spacing.

@Keavon Keavon changed the title Add maximum width to text tool Add maximum width to text boxes and the Text tool Dec 21, 2024
@Keavon
Copy link
Member

Keavon commented Dec 23, 2024

I'll set this to draft status while waiting on those changes. @Nitish-bot, please mark it was ready for review and ping me in a comment when you've got those resolved. Thank you!

@Keavon Keavon marked this pull request as draft December 23, 2024 08:05
Copy link

📦 Build Complete for ad2a927
https://2fb0e5c4.graphite.pages.dev

@Nitish-bot Nitish-bot marked this pull request as ready for review December 31, 2024 20:16
@Keavon Keavon changed the title Add maximum width to text boxes and the Text tool Add max width/height to text layers and draggable text boxes to the Text tool Jan 1, 2025
@Keavon
Copy link
Member

Keavon commented Jan 1, 2025

Needed follow-up work:

  • Fix the max height parameter so it actually cuts off any lines which exceed that height by any amount.
  • Visually indicate with a red overlay along the bottom if text is being cut off when in the Select and Text tools.
  • Store the max width and max height parameter values, when unchecked, to hidden parameters serving the purpose of backing up those numbers to be restored upon reenabling them— or alternatively, create a new data type called something like RestorableOption<T> with Some(T) and None(T) variants, where the latter backs up the disabled value.
  • When placing or dragging with the Text tool (when the left mouse button is down, and the pointer either hasn't been dragged yet or it is being dragged), add support for right-click and Esc to cancel and return to the Ready state.
  • Draggable edge/corner handles (similar to the Select tool's transform cage, but only scaling and not rotation or translation) around the text area box shown by the Text tool.

@Keavon Keavon merged commit 6635754 into GraphiteEditor:master Jan 1, 2025
3 of 4 checks passed
@Keavon Keavon mentioned this pull request Jan 1, 2025
16 tasks
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