Skip to content

Conversation

@VladislavAntonyuk
Copy link
Collaborator

@VladislavAntonyuk VladislavAntonyuk commented Jan 19, 2025

Description of Change

Linked Issues

PR Checklist

Additional information

Screenshot 2025-01-19 at 18 44 28 Screenshot 2025-01-19 at 18 45 44 Screenshot 2025-01-19 at 18 46 32

@VladislavAntonyuk VladislavAntonyuk self-assigned this Jan 19, 2025
Copilot AI review requested due to automatic review settings January 19, 2025 16:43
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot reviewed 2 out of 3 changed files in this pull request and generated no comments.

Files not reviewed (1)
  • samples/CommunityToolkit.Maui.Sample/Pages/Alerts/SnackbarPage.xaml: Language not supported
Comments suppressed due to low confidence (2)

src/CommunityToolkit.Maui.Core/Views/Alert/AlertView.macios.cs:60

  • Ensure that the new layout behavior introduced in LayoutSubviews is covered by tests.
public override void LayoutSubviews()

samples/CommunityToolkit.Maui.Sample/Pages/Alerts/SnackbarPage.xaml.cs:85

  • [nitpick] The method name 'DisplayCustomSnackbarButtonClicked2' is ambiguous and should be renamed to something more descriptive, such as 'DisplayCustomSnackbarWithOptions'.
async void DisplayCustomSnackbarButtonClicked2(object sender, EventArgs e)

@VladislavAntonyuk VladislavAntonyuk enabled auto-merge (squash) January 19, 2025 16:43
Copy link
Collaborator

@TheCodeTraveler TheCodeTraveler left a comment

Choose a reason for hiding this comment

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

Thanks Vlad - this looks great! 💯

Just one problem we should fix before merging: The app crashes when a Popup with an anchor is displayed and while it is currently displayed we navigate away from the page.

Here's two examples demonstrating this using the Sample App:

ScreenFlow

ScreenFlow

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot reviewed 5 out of 7 changed files in this pull request and generated no comments.

Files not reviewed (2)
  • samples/CommunityToolkit.Maui.Sample/Pages/Alerts/SnackbarPage.xaml: Language not supported
  • src/CommunityToolkit.Maui.Core/Views/Toast/PlatformToast.macios.cs: Evaluated as low risk

Copy link
Contributor

@ne0rrmatrix ne0rrmatrix left a comment

Choose a reason for hiding this comment

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

Looks very well done! Just need to fix that IOS bug and it will be good to merge. The issue was listed previously by the @TheCodeTraveler. Once that is cleared i will be happy to approve this. The look and feel is now consistant across both android and ios.

Copy link
Contributor

@ne0rrmatrix ne0rrmatrix 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. Good I have tested them on IOS and the crash is no longer an issue. Only one minor bug that might be nice to fix before merge.

2025-01-28.09-37-05.mp4

@VladislavAntonyuk VladislavAntonyuk merged commit d8cf48d into main Feb 4, 2025
10 checks passed
@VladislavAntonyuk VladislavAntonyuk deleted the 1901-fix-snackbar branch February 4, 2025 11:42
@github-actions github-actions bot locked and limited conversation to collaborators Feb 6, 2025
@VladislavAntonyuk VladislavAntonyuk added the needs discussion Discuss it on the next Monthly standup label Feb 6, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

needs discussion Discuss it on the next Monthly standup

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Snackbar layout looks different on iOS and Android

3 participants