Skip to content
This repository has been archived by the owner on May 1, 2024. It is now read-only.

GH-3106 Implemented LineBreakMode to Button #11147

Merged
merged 9 commits into from
Sep 1, 2020
Merged

Conversation

pictos
Copy link
Contributor

@pictos pictos commented Jun 20, 2020

Description of Change

I created this one because the #7810 was very old and painful to do a rebase.

Added the LineBreakMode to Button.

Issues Resolved

API Changes

Added:

  • LineBreakMode Button.LineBreakMode { get; set; } //Bindable Property

Platforms Affected

  • Core/XAML (all platforms)
  • iOS
  • Android
  • UWP

Behavioral/Visual Changes

Now users can set a line break mode to texts on Button, the default value doesn't affect any user.

None

Before/After Screenshots

SampleForForms

PR Checklist

  • Targets the correct branch
  • Tests are passing (or failures are unrelated)

return;

view.SetLineBreakMode(_element);
_renderer.View.SetAllCaps(_element.TextTransform == TextTransform.Default);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@samhouts I used the same approach used here, to preserve the text as AllCaps.

Co-authored-by: Stephane Delcroix <stephane@delcroix.org>
@samhouts samhouts changed the base branch from master to main June 25, 2020 23:58
@samhouts samhouts added this to In Review in .NET MAUI Backlog Jul 30, 2020
@riverar
Copy link

riverar commented Aug 13, 2020

When resizing a UWP app window using MiddleTruncation or HeadTruncation, the entire string is truncated, either to the first character or first available space (see screenshot).

Test cases

<Button Text="HeadTruncationHeadTruncationHeadTruncationHeadTruncationHeadTruncation" LineBreakMode="HeadTruncation"/>
<Button Text="MiddleTruncationMiddleTruncationMiddleTruncationMiddleTruncationMiddleTruncation" LineBreakMode="MiddleTruncation"/>
<Button Text="TailTruncationTailTruncationTailTruncationTailTruncationTailTruncationTailTruncation" LineBreakMode="TailTruncation"/>

image

Copy link

@riverar riverar left a comment

Choose a reason for hiding this comment

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

Changes look OK, but the above bug spoils the UWP experience. Thoughts on how to proceed here? @StephaneDelcroix @nickrandolph

I'm inclined to reject until the bug is resolved.

textBlock.TextWrapping = TextWrapping.Wrap;
break;
case LineBreakMode.HeadTruncation:
// TODO: This truncates at the end.
Copy link

Choose a reason for hiding this comment

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

Nit: Remove TODO

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@riverar, as you can see here those comments exist before my change, I just moved all the code around. I don't think this is a good idea to remove it.

Copy link
Member

Choose a reason for hiding this comment

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

.....and I think I may have added those myself 😅

DetermineTruncatedTextWrapping(textBlock);
break;
case LineBreakMode.MiddleTruncation:
// TODO: This truncates at the end.
Copy link

Choose a reason for hiding this comment

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

Nit: Remove TODO

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@riverar, as you can see here those comments exist before my change, I just moved all the code around. I don't think this is a good idea to remove it.

@samhouts
Copy link
Member

Yep, I agree, that UWP bug seems a bit nasty. @pictos Wanna take a look at that?

@pictos
Copy link
Contributor Author

pictos commented Aug 13, 2020

@samhouts, this bug isn't related to this PR, you can see it here #5284 for UWP (we have some know-bugs for Android also). I can try to solve this bug (and others), but I think it would be better to address other PR for it. Does that make sense to you?

@samhouts
Copy link
Member

@pictos Very good! Thanks for clarifying!

@samhouts samhouts added API-change Heads-up to reviewers that this PR may contain an API change approved Has two approvals, no pending reviews, and no changes requested labels Aug 17, 2020
@samhouts
Copy link
Member

@pictos It looks like this may have broken this test:
image

Are you able to run that locally?

@samhouts
Copy link
Member

More information about the failing test: Android Fast Renderers only

System.Exception : Error while performing WaitForElement(Marked("Page1"), "Timed out waiting for element...", null, null, null)
----> System.TimeoutException : Timed out waiting for element...

@riverar
Copy link

riverar commented Aug 21, 2020

image

Waiting for Microsoft resolution on this one.

@pictos
Copy link
Contributor Author

pictos commented Aug 21, 2020

@samhouts, if the test MemoryLeakB42329 is the only one that fails, can we assume that the failure is unrelated with this PR? (of course, if the test pass locally)

@samhouts samhouts merged commit 17881ec into xamarin:main Sep 1, 2020
.NET MAUI Backlog automation moved this from In Review to Done Sep 1, 2020
@pictos pictos deleted the gh-3106 branch September 1, 2020 22:19
rmarinho pushed a commit that referenced this pull request Sep 2, 2020
* Added LineBreakMode in the Button and TestAttributes

* Added LineBreakMode implementation on Android platform

* Added Issue into the Controls project

* Removed unused method

* Added UWP support for LineBreakMode

* Implemented LineBreakMode on iOS

* Update Xamarin.Forms.Core/Button.cs

Co-authored-by: Stephane Delcroix <stephane@delcroix.org>

Co-authored-by: Stephane Delcroix <stephane@delcroix.org>
Co-authored-by: Rui Marinho <me@ruimarinho.net>

fixes #3106
@samhouts samhouts added this to Done in Sprint 175 Sep 2, 2020
pictos added a commit to pictos/Xamarin.Forms that referenced this pull request Sep 12, 2020
* Added LineBreakMode in the Button and TestAttributes

* Added LineBreakMode implementation on Android platform

* Added Issue into the Controls project

* Removed unused method

* Added UWP support for LineBreakMode

* Implemented LineBreakMode on iOS

* Update Xamarin.Forms.Core/Button.cs

Co-authored-by: Stephane Delcroix <stephane@delcroix.org>

Co-authored-by: Stephane Delcroix <stephane@delcroix.org>
Co-authored-by: Rui Marinho <me@ruimarinho.net>

fixes xamarin#3106
domagojmedo pushed a commit to domagojmedo/Xamarin.Forms that referenced this pull request Sep 12, 2020
* Added LineBreakMode in the Button and TestAttributes

* Added LineBreakMode implementation on Android platform

* Added Issue into the Controls project

* Removed unused method

* Added UWP support for LineBreakMode

* Implemented LineBreakMode on iOS

* Update Xamarin.Forms.Core/Button.cs

Co-authored-by: Stephane Delcroix <stephane@delcroix.org>

Co-authored-by: Stephane Delcroix <stephane@delcroix.org>
Co-authored-by: Rui Marinho <me@ruimarinho.net>

fixes xamarin#3106
pictos added a commit to pictos/Xamarin.Forms that referenced this pull request Sep 12, 2020
* Added LineBreakMode in the Button and TestAttributes

* Added LineBreakMode implementation on Android platform

* Added Issue into the Controls project

* Removed unused method

* Added UWP support for LineBreakMode

* Implemented LineBreakMode on iOS

* Update Xamarin.Forms.Core/Button.cs

Co-authored-by: Stephane Delcroix <stephane@delcroix.org>

Co-authored-by: Stephane Delcroix <stephane@delcroix.org>
Co-authored-by: Rui Marinho <me@ruimarinho.net>

fixes xamarin#3106
jsuarezruiz pushed a commit that referenced this pull request Sep 17, 2020
* Added LineBreakMode in the Button and TestAttributes

* Added LineBreakMode implementation on Android platform

* Added Issue into the Controls project

* Removed unused method

* Added UWP support for LineBreakMode

* Implemented LineBreakMode on iOS

* Update Xamarin.Forms.Core/Button.cs

Co-authored-by: Stephane Delcroix <stephane@delcroix.org>

Co-authored-by: Stephane Delcroix <stephane@delcroix.org>
Co-authored-by: Rui Marinho <me@ruimarinho.net>

fixes #3106
@CartBlanche
Copy link

I'm using 5.0.0.1791-pre5 and LineBreakMode on Buttons, does not appear to exist?

shyunMin pushed a commit to shyunMin/Xamarin.Forms that referenced this pull request Feb 19, 2021
* Added LineBreakMode in the Button and TestAttributes

* Added LineBreakMode implementation on Android platform

* Added Issue into the Controls project

* Removed unused method

* Added UWP support for LineBreakMode

* Implemented LineBreakMode on iOS

* Update Xamarin.Forms.Core/Button.cs

Co-authored-by: Stephane Delcroix <stephane@delcroix.org>

Co-authored-by: Stephane Delcroix <stephane@delcroix.org>
Co-authored-by: Rui Marinho <me@ruimarinho.net>

fixes xamarin#3106
@Juansero29
Copy link

Why is the API for LineBreakMode still not included in Xamarin.Forms 5.0.0.2012? When is this being included?

@riverar
Copy link

riverar commented Feb 28, 2021

@samhouts Heads up ^

@pictos
Copy link
Contributor Author

pictos commented Feb 28, 2021

cc @Redth

Axemasta pushed a commit to Axemasta/Xamarin.Forms that referenced this pull request Apr 5, 2021
* Added LineBreakMode in the Button and TestAttributes

* Added LineBreakMode implementation on Android platform

* Added Issue into the Controls project

* Removed unused method

* Added UWP support for LineBreakMode

* Implemented LineBreakMode on iOS

* Update Xamarin.Forms.Core/Button.cs

Co-authored-by: Stephane Delcroix <stephane@delcroix.org>

Co-authored-by: Stephane Delcroix <stephane@delcroix.org>
Co-authored-by: Rui Marinho <me@ruimarinho.net>

fixes xamarin#3106
@toddanglin
Copy link

@pictos @Redth @samhouts Bump. Can you add any insight in to the timing this enhancement will be available in Xamarin Forms? Still does not appear to be available as of 5.0.0.2291.

@Pastajello

This comment was marked as off-topic.

@riverar
Copy link

riverar commented May 13, 2022

@samhouts Seems this change never made it out, are you tracking this?

@pictos
Copy link
Contributor Author

pictos commented May 13, 2022

cc: @jfversluis

@jfversluis
Copy link
Member

jfversluis commented May 17, 2022

Seems this was merged into main while we are releasing from 5.0.0 for a while. But this happened while I was not on the team so I can't give you the full context. Unfortunately, we don't really have a way for this to add this to Forms anymore as we're in maintanance mode and don't want to add any new APIs unless we really need to to support new platform OS features :(

.NET MAUI was branched of main which does have all the changed including this one. So I guess it's time to update to .NET MAUI!

@pictos
Copy link
Contributor Author

pictos commented May 17, 2022

Thanks for the reply

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
API-change Heads-up to reviewers that this PR may contain an API change approved Has two approvals, no pending reviews, and no changes requested Core F100 p/Android p/iOS 🍎 p/UWP proposal-open t/enhancement ➕
Projects
Sprint 175
  
Done
Development

Successfully merging this pull request may close these issues.

[Enhancement] LineBreakMode for Button