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 android toolbar icon change #20166

Closed
wants to merge 6 commits into from

Conversation

kubaflo
Copy link
Contributor

@kubaflo kubaflo commented Jan 26, 2024

Description of Change

Fixed android toolbar icon change on shell pages

Issues Fixed

Fixes #19673
Fixes #19950

Screen.Recording.2024-01-26.at.01.17.15.mov

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

ghost commented Jan 26, 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
Copy link
Contributor

Added UI Test.

Copy link
Contributor

Choose a reason for hiding this comment

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

This snapshot must be updated with the one generated by CI.

.MoveTo(endX, endY)
.Wait(swipeSpeed)
Copy link
Contributor

Choose a reason for hiding this comment

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

Changes in the order of the actions to avoid a DriverException: Method has not yet been implemented.

xmlns:ns="clr-namespace:Maui.Controls.Sample.Issues"
xmlns:android="clr-namespace:Microsoft.Maui.Controls.PlatformConfiguration.AndroidSpecific;assembly=Microsoft.Maui.Controls"
android:TabbedPage.ToolbarPlacement="Bottom">
<local:Issue19673Page1 Title="Page 1" AutomationId="Page1" />
Copy link
Contributor

Choose a reason for hiding this comment

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

Test based on the sample from #19673

@kubaflo kubaflo changed the title Fixed android toolbar icon change (#19673) Fixed android toolbar icon change Jan 28, 2024
@rmarinho
Copy link
Member

rmarinho commented Feb 1, 2024

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@PureWeen
Copy link
Member

PureWeen commented Feb 5, 2024

Similar PR here #16796

@dustin-wojciechowski
Copy link
Contributor

It would be fantastic if we also had a UI Test that matched the example provided in the description, it could even be a second one that covers #19950, that is, different icons and text being displayed. As seen below, it looks like the test is checking if one icon set on one page is persisting across the others.

19673.webm

@kubaflo kubaflo force-pushed the android-toolbar-icon-update-fix branch from 0a31c4b to d603b16 Compare April 27, 2024 23:39
@kubaflo
Copy link
Contributor Author

kubaflo commented Apr 27, 2024

@dustin-wojciechowski I've added a Ui Test that covers the #19950 bug like you asked for :)

@PureWeen
Copy link
Member

PureWeen commented May 2, 2024

/rebase

@github-actions github-actions bot force-pushed the android-toolbar-icon-update-fix branch from d603b16 to cb02b5b Compare May 2, 2024 18:33
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@jfversluis
Copy link
Member

/rebase

@jfversluis
Copy link
Member

@kubaflo @PureWeen do we still need this now we merged #16796?

@kubaflo
Copy link
Contributor Author

kubaflo commented May 29, 2024

@jfversluis yes, I don't think we need it anymore. Therefore, I'm closing this pr :)

@kubaflo kubaflo closed this May 29, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Jun 29, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
7 participants