Skip to content

FIX: Made text alignment work when max width is disabled#4025

Open
Chetansahney wants to merge 2 commits intoGraphiteEditor:masterfrom
Chetansahney:clean-text-alignment-fix
Open

FIX: Made text alignment work when max width is disabled#4025
Chetansahney wants to merge 2 commits intoGraphiteEditor:masterfrom
Chetansahney:clean-text-alignment-fix

Conversation

@Chetansahney
Copy link
Copy Markdown

Fixes #4011

This PR fixes the issue where text alignment settings (Center/Right/Justify) were ignored unless a Max Width was set. I've updated the logic so that the text block now calculates its own width based on the longest line, allowing alignment to work correctly relative to the text's own bounding box.

Key Changes

Enabled alignment support for text nodes without a defined Max Width.
Updated the layout engine to use the dynamic width of the text rows as the alignment basis.

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 modifies the text alignment logic in text_context.rs to ensure alignment works correctly when a maximum width is not specified, by falling back to the layout's full width. The review feedback suggests using the more idiomatic or_else method for the fallback value and correcting a minor spacing issue in a code comment.

Comment on lines +85 to +87
//To make text alignment work when the max width is disabled
let alignment_width = typesetting.max_width.map(|max_w| max_w as f32).or(Some(layout.full_width()));
layout.align(alignment_width, typesetting.align.into(), AlignmentOptions::default());
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

The comment is missing a space after the slashes. Additionally, while layout.full_width() is likely inexpensive, using or_else is a more idiomatic way to provide a fallback value in Rust as it avoids unnecessary evaluation when max_width is present.

Suggested change
//To make text alignment work when the max width is disabled
let alignment_width = typesetting.max_width.map(|max_w| max_w as f32).or(Some(layout.full_width()));
layout.align(alignment_width, typesetting.align.into(), AlignmentOptions::default());
// To make text alignment work when the max width is disabled
let alignment_width = typesetting.max_width.map(|max_w| max_w as f32).or_else(|| Some(layout.full_width()));
layout.align(alignment_width, typesetting.align.into(), AlignmentOptions::default());

Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

No issues found across 1 file

Confidence score: 5/5

  • Automated review surfaced no issues in the provided summaries.
  • No files require special attention.

@Annonnymmousss
Copy link
Copy Markdown
Contributor

@Chetansahney It would be helpful if you could attach the before after video clips. Thankyou.

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.

Text alignment should work without an explicit max-width

2 participants