[Android] The number of SearchHandler toolbar item increases abnormally - fix #29561
Conversation
kubaflo
added
platform/android 🤖
community ✨
There was a problem hiding this comment.
Pull Request Overview
This PR addresses an issue where the SearchHandler toolbar item would be added repeatedly on Android by assigning a fixed placeholder ID and separating tint updates from item updates.
- Replaces the generic
UpdateToolbarItemscall in theTintColorsetter with a newUpdateToolbarItemsTintColorsmethod to avoid re-adding items. - Introduces a hard-coded placeholder menu item ID (
100) when adding the search placeholder to prevent duplicates. - Adds two overloads of
UpdateToolbarItemsTintColors(one private and one parameterless) to apply tint without touching item setup.
Comments suppressed due to low confidence (1)
src/Controls/src/Core/Compatibility/Handlers/Shell/Android/ShellToolbarTracker.cs:634
- Magic number 100 is used for the placeholder menu item ID in two places; consider extracting it into a named constant to avoid duplication and improve maintainability.
int _placeholderMenuItemId = 100;
| if (menu.FindItem(_placeholderMenuItemId) is IMenuItem item) | ||
| { | ||
| using (var icon = item.Icon) | ||
| icon.SetColorFilter(TintColor.ToPlatform(Colors.White), FilterMode.SrcAtop); |
There was a problem hiding this comment.
The color filter is hard-coded to white (using Colors.White) instead of the actual TintColor; you should apply the intended TintColor (e.g., TintColor.ToPlatform()) to respect the configured tint.
| icon.SetColorFilter(TintColor.ToPlatform(Colors.White), FilterMode.SrcAtop); | |
| icon.SetColorFilter((TintColor?.ToPlatform() ?? Colors.White.ToPlatform()), FilterMode.SrcAtop); |
Copilot uses AI. Check for mistakes.
|
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. |
|
/azp run MAUI-UITests-public |
|
Azure Pipelines successfully started running 1 pipeline(s). |
| App.Click("GoBackButton"); | ||
| App.WaitForElement("GotoIssue29499Subpage"); | ||
| ((AppiumApp)App).Driver.FindElement(OpenQA.Selenium.By.XPath("//*[@content-desc=\"More options\"]")).Click(); | ||
| VerifyScreenshot(); |
There was a problem hiding this comment.
Pending snapshots, running a Build.
There was a problem hiding this comment.
Could you commit the image?
Note
Are you waiting for the changes in this PR to be merged?
It would be very helpful if you could test the resulting artifacts from this PR and let us know in a comment if this change resolves your issue. Thank you!
Issues Fixed
Fixes #29499
Screen.Recording.2025-05-17.at.02.17.05.mov
Screen.Recording.2025-05-17.at.02.18.30.mov