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 start end end to the TextHorizontalAlignment #4550

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ogoffart
Copy link
Member

@ogoffart ogoffart commented Feb 6, 2024

RTL languages are not supported yet though, but once they will it will be usefull

RTL languages are not supported yet though, but once they will it will
be usefull
@ogoffart
Copy link
Member Author

ogoffart commented Feb 6, 2024

The idea is that people would start now by using the right value for the enum even if it doesn't work yet.
But is it a good idea to already add it when right to left languages are not working yet?

Copy link
Member

@tronical tronical left a comment

Choose a reason for hiding this comment

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

I think it makes sense.

@@ -22,6 +22,12 @@ macro_rules! for_each_enums {
$macro![
/// This enum describes the different types of alignment of text along the horizontal axis of a [`Text`](elements.md#text) element.
enum TextHorizontalAlignment {
/// The text will be aligned with the start edge of the containing box.
/// This could be left or right depending on the direction of the text in the language.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
/// This could be left or right depending on the direction of the text in the language.
/// This could be left or right depending on the direction of the text.

/// This could be left or right depending on the direction of the text in the language.
Start,
/// The text will be aligned with the end edge of the containing box.
/// This could be left or right depending on the direction of the text in the language.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
/// This could be left or right depending on the direction of the text in the language.
/// This could be left or right depending on the direction of the text.

@@ -97,9 +97,13 @@ pub fn create_layout(
}

style.set_text_align(match h_align {
items::TextHorizontalAlignment::Left => skia_safe::textlayout::TextAlign::Left,
TextHorizontalAlignment::Start | items::TextHorizontalAlignment::Left => {
Copy link
Member

Choose a reason for hiding this comment

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

I think start should be mapped to skia_safe::textlayout::TextAlign::Start; same for end.

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