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

[BUG] Android popup. Layout breaks if data is populated after the popup is shown. #1423

Closed
2 tasks done
Hooterr opened this issue Sep 22, 2023 · 7 comments · Fixed by #1361
Closed
2 tasks done

[BUG] Android popup. Layout breaks if data is populated after the popup is shown. #1423

Hooterr opened this issue Sep 22, 2023 · 7 comments · Fixed by #1361
Labels
bug Something isn't working

Comments

@Hooterr
Copy link

Hooterr commented Sep 22, 2023

Is there an existing issue for this?

  • I have searched the existing issues

Did you read the "Reporting a bug" section on Contributing file?

Current Behavior

Populating labels after the popup is shown causes layout issues on Android. This is a huge issue if you need to use any sort of binding inside the popup.

When the labels are populated immediately

Screenshot_1695383002

When the labels are populated a split second later, by a binding for isntance.
Screenshot_1695383089

As you can see the longer labels don't split into multiple lines correctly, or the single line label doesn't respect HorizontalTextAlignment. I have tried couple of layouts including Grid and StackLayout or ScrollView but no combination fixes the issue.

Expected Behavior

Changing / Setting text values for labels or other text based controls shouldn't break layouts inside the popup.

Steps To Reproduce

  1. Show a popup
  2. After it's visible set Text values for the labels

Link to public reproduction project repository

https://github.com/Hooterr/PopupLayoutIssue

Environment

- .NET MAUI CommunityToolkit: 5.3.0
- OS: Macos
- .NET MAUI: 7.0.92

Anything else?

No response

@Hooterr Hooterr added bug Something isn't working unverified labels Sep 22, 2023
@cat0363
Copy link
Contributor

cat0363 commented Sep 26, 2023

This is caused by the following process added to redraw the Popup when the screen is rotated.
I'm writing it here for my personal notes.

[src\CommunityToolkit.Maui.Core\Handlers\Popup\PopUpHandler.android.cs]

void OnLayoutChange(object? sender, EventArgs e)
{
    if (VirtualView?.Handler?.PlatformView is Dialog dialog && Container is not null)
    {
        PopupExtensions.SetSize(dialog, VirtualView, Container);
    }
}

Abolishing the above call will resolve this issue, but another problem will occur because redrawing
will not occur when the screen is rotated.

@cat0363
Copy link
Contributor

cat0363 commented Sep 27, 2023

The PR #1361 I created should resolve this issue.

Android.Emulator.-.pixel_2_-_api_30_5554.2023-09-27.14-02-55.mp4

I thought this was an unresolved issue and was investigating it separately,
but it is scheduled to be resolved in PR #1361.There was no problem with screen rotation,
which I was concerned about.

@cat0363
Copy link
Contributor

cat0363 commented Sep 27, 2023

It looks like PR #1361 needs further fixes.
If you look closely at the video, the position of the label for Test123 is different.

@Hooterr
Copy link
Author

Hooterr commented Sep 27, 2023

As a workaround just put WidthRequest="300" (or whatever your number is) on the top most layout in your popup.
Probably will break when the device is rotated, but at least you can use it now.

@brminnick
Copy link
Collaborator

@cat0363 Just to confirm - does PR #1361 now fix this Issue? Or will we need a separate PR to fix this Issue?

@cat0363
Copy link
Contributor

cat0363 commented Sep 30, 2023

@brminnick , I resolved this issue with PR #1361.
There is no need to create a new PR. FYI, I wrote it in the post below.
#1361 (comment)

Thank you for your review of PR #1361.

@brminnick
Copy link
Collaborator

brminnick commented Sep 30, 2023

Thanks for confirming!

Fixed in #1423

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants