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

Fixed line break mode for buttons #20133

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

kubaflo
Copy link
Contributor

@kubaflo kubaflo commented Jan 24, 2024

Description of Change

Issues Fixed

Fixes #19806

Before:

before.mov

After:

after.mov

@kubaflo kubaflo requested a review from a team as a code owner January 24, 2024 14:12
@ghost ghost added the community ✨ Community Contribution label Jan 24, 2024
@ghost
Copy link

ghost commented Jan 24, 2024

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

@jsuarezruiz jsuarezruiz added the area-layout StackLayout, GridLayout, ContentView, AbsoluteLayout, FlexLayout, ContentPresenter label Jan 24, 2024
kubaflo added a commit to kubaflo/maui that referenced this pull request Jan 25, 2024
@kubaflo kubaflo changed the title Update LayoutExtensions.cs Fixed line break mode for buttons Jan 26, 2024
@albyrock87
Copy link
Contributor

MAUI/Microsoft me, this gentleman and other developers are spending a lot of time for free trying to help you improve the platform and fix bugs.

This PR seems ready to be merged or at least reviewed since three weeks ago. I have another PR waiting since October, and many other developers are on the same situation.

If MAUI team needs more devs, please tell Microsoft to hire new developers.

Now, besides the complains, can we move on with this PR? Thanks.

@jsuarezruiz
Copy link
Contributor

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

Copy link
Member

@rmarinho rmarinho left a comment

Choose a reason for hiding this comment

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

Hey , thanks for your help, seems this changes to the LayoutExtensions are causing some unit tests to failing

https://dev.azure.com/xamarin/public/_build/results?buildId=109492&view=ms.vss-test-web.build-test-results-tab

kubaflo added a commit to kubaflo/maui that referenced this pull request Feb 22, 2024
kubaflo added a commit to kubaflo/maui that referenced this pull request Feb 22, 2024
@kubaflo
Copy link
Contributor Author

kubaflo commented Feb 22, 2024

@rmarinho I've fixed the tests
Screenshot 2024-02-22 at 17 40 43

@hansmbakker
Copy link

@rmarinho / @albyrock87 / @StephaneDelcroix could you please have a new look at @kubaflo's code? He updated it based on your feedback, and it would really be helpful to have this issue fixed.

@mattleibow
Copy link
Member

Please reabse as well. The tests are very red and we think we fixed some more tests recently. Hopefully. CI is a bit dodgy.

kubaflo added a commit to kubaflo/maui that referenced this pull request Mar 18, 2024
kubaflo added a commit to kubaflo/maui that referenced this pull request Mar 18, 2024
@kubaflo kubaflo force-pushed the lineBreakMode-in-button-fix branch 2 times, most recently from 9332090 to ffad8ef Compare March 18, 2024 18:53
@PureWeen
Copy link
Member

/rebase

@github-actions github-actions bot force-pushed the lineBreakMode-in-button-fix branch from ffad8ef to cf8ee1e Compare April 15, 2024 07:02
@PureWeen
Copy link
Member

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@@ -45,6 +45,11 @@ public static Rect ComputeFrame(this IView view, Rect bounds)
{
consumedWidth = Math.Min(bounds.Width, view.MaximumWidth);
}
else if (!IsExplicitSet(view.Width))
Copy link
Contributor

@albyrock87 albyrock87 Apr 19, 2024

Choose a reason for hiding this comment

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

@kubaflo we have the same logical bug on the vertical constraint, would you mind fixing that too?

This for example impacts while using a portrait image with AspectFit put inside a smaller vertical space with VerticalPosition="Start": the image will overflow the container (which is wrong).

Thanks

			// But, if the element is set to fill vertically and it doesn't have an explicitly set height,
			// then we want the minimum between its MaximumHeight  and the bounds' height
			// MaximumHeight is always positive infinity if not defined by the user
			if (view.VerticalLayoutAlignment == LayoutAlignment.Fill && !IsExplicitSet(view.Height))
			{
				consumedHeight = Math.Min(bounds.Height, view.MaximumHeight);
			}
// ->
            else if (!IsExplicitSet(view.Height))
			{
				consumedHeight = Math.Min(bounds.Height, view.DesiredSize.Height);
			}

Copy link
Member

Choose a reason for hiding this comment

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

@albyrock87 do you perhaps have a sample of this issue? I just want to make sure that we get a test in for your issue either in here or in another PR. I will have a look at what you are seeing and try make a repro, but if you have something it will help.

Copy link
Contributor

@albyrock87 albyrock87 May 29, 2024

Choose a reason for hiding this comment

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

@jsuarezruiz
Copy link
Contributor

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@jsuarezruiz jsuarezruiz added the area-controls-button Button, ImageButton label May 13, 2024
@jsuarezruiz
Copy link
Contributor

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@mattleibow
Copy link
Member

/azp run

@mattleibow
Copy link
Member

Hopefully I rebased correctly

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@kubaflo
Copy link
Contributor Author

kubaflo commented May 29, 2024

@mattleibow Thanks!

@mattleibow
Copy link
Member

/azp run

@mattleibow
Copy link
Member

I had a bad merge there... Hopefully this fixes it.

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@mattleibow
Copy link
Member

@kubaflo not sure if you say #20133 (comment)

@jsuarezruiz
Copy link
Contributor

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@kubaflo
Copy link
Contributor Author

kubaflo commented Jun 11, 2024

@kubaflo not sure if you say #20133 (comment)

I think a separate PR would be better for it

@jsuarezruiz
Copy link
Contributor

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@jsuarezruiz
Copy link
Contributor

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-controls-button Button, ImageButton area-layout StackLayout, GridLayout, ContentView, AbsoluteLayout, FlexLayout, ContentPresenter community ✨ Community Contribution
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[iOS] Button.LineBreakMode Does Not Appear to Truncate in this Context: StackLayout > Border > Grid > Button
7 participants