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 support for TextAlignment.Justify - fix #24376

Merged

Conversation

MartyIX
Copy link
Contributor

@MartyIX MartyIX commented Aug 22, 2024

Description of Change

Adds support for TextAlignment.Justify (or should it be Justified?).

  • Android
  • iOS/macOS
  • Tizen
  • Windows

Demo

Android

image

Windows

image

iOS

image

Issues Fixed

Fixes #24373

@dotnet-policy-service dotnet-policy-service bot added the community ✨ Community Contribution label Aug 22, 2024
Copy link
Contributor

Hey there @MartyIX! Thank you so much for your PR! Someone from the team will get assigned to your PR shortly and we'll get it reviewed.

@MartyIX MartyIX marked this pull request as ready for review August 22, 2024 12:29
@MartyIX MartyIX requested a review from a team as a code owner August 22, 2024 12:29
}
TextAlignment.Center => HorizontalAlignment.Center,
TextAlignment.End => HorizontalAlignment.Right,
TextAlignment.Justify => HorizontalAlignment.Stretch,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

New line

@@ -8,6 +8,7 @@ public enum TextAlignment
/// <include file="../../docs/Microsoft.Maui/TextAlignment.xml" path="//Member[@MemberName='Center']/Docs/*" />
Center,
/// <include file="../../docs/Microsoft.Maui/TextAlignment.xml" path="//Member[@MemberName='End']/Docs/*" />
End
End,
Justify
Copy link
Contributor Author

Choose a reason for hiding this comment

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

API change

Comment on lines +16 to +17
case Microsoft.UI.Xaml.TextAlignment.Justify:
return HorizontalAlignment.Stretch;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not entirely sure if I should do this or not.

@MartyIX MartyIX force-pushed the feature/2024-08-22-TextAlignment-Justify branch from b233b4b to 611ebec Compare August 22, 2024 12:46
@PureWeen
Copy link
Member

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@@ -40,6 +41,13 @@ public static void UpdateTextAlignment(this EditText view, TextAlignment horizon
view.TextAlignment = horizontal.ToTextAlignment();
view.Gravity = vertical.ToVerticalGravityFlags();
}

if (OperatingSystem.IsAndroidVersionAtLeast(26))
Copy link
Contributor

@albyrock87 albyrock87 Aug 22, 2024

Choose a reason for hiding this comment

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

This method is not being used inside MAUI and I would evaluate removing it from PublicAPI (or at least mark it as obsolete and remove with .NET10).

public static void UpdateTextAlignment(this EditText view, TextAlignment horizontal, TextAlignment vertical)

Besides that the goal here is to support text alignment on Label right?
If so, this code should be moved here: https://github.com/dotnet/maui/blob/main/src/Core/src/Platform/Android/TextViewExtensions.cs#L54

If you want to also support Entry text alignment (I guess so), you can use the above method

internal static void UpdateHorizontalAlignment(this EditText view, TextAlignment alignment, AGravityFlags orMask = AGravityFlags.NoGravity)

Copy link
Member

Choose a reason for hiding this comment

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

Yea, I tested moving this into EditText as well but it didn't seem to do anything to EditText unfortunately :-(

Copy link
Member

Choose a reason for hiding this comment

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

alright marked this one obsolete and removed non functional code

Foda
Foda previously approved these changes Aug 22, 2024
@rmarinho
Copy link
Member

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@PureWeen
Copy link
Member

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

PureWeen
PureWeen previously approved these changes Aug 23, 2024
Copy link
Member

@PureWeen PureWeen left a comment

Choose a reason for hiding this comment

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

  • I tested this on android/ios and it worked great
  • @Foda validated on WinUI
  • @MartyIX is going to add tests in a follow up PR soon to come!

@PureWeen
Copy link
Member

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@PureWeen PureWeen added this to the 9.0-rc1 milestone Aug 24, 2024
@PureWeen
Copy link
Member

/rebase

@github-actions github-actions bot force-pushed the feature/2024-08-22-TextAlignment-Justify branch from 9e9051d to 557371b Compare August 24, 2024 18:26
@PureWeen
Copy link
Member

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@PureWeen PureWeen merged commit 4051892 into dotnet:net9.0 Aug 24, 2024
117 checks passed
@samhouts samhouts added the fixed-in-net9.0-nightly This may be available in a nightly release! label Aug 27, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Oct 6, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
community ✨ Community Contribution fixed-in-net9.0-nightly This may be available in a nightly release!
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

6 participants