[Issue-Resolver] Add ShowsCancelButton property to SearchBar and SearchHandler #33010
Conversation
Fixes dotnet#33008 - Adds ShowsCancelButton property to both SearchBar and SearchHandler - Defaults to true, fixing iOS keyboard trap issue - Removes hardcoded ShowsCancelButton = false from SearchHandlerAppearanceTracker - Adds device tests verifying property behavior - Platform-specific: iOS/MacCatalyst only, no-op on Android/Windows - Backward compatible, no breaking changes
|
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. |
kubaflo
force-pushed
the
fix-issue-33008-searchbar-cancel-button3
branch
from
609eadd to
4b4d992
Compare
There was a problem hiding this comment.
Pull request overview
This PR adds a new ShowsCancelButton property to both SearchBar and SearchHandler to address a keyboard trap issue on iOS where users couldn't dismiss the keyboard. The property defaults to true and is only functional on iOS/MacCatalyst platforms.
Key changes:
- Added
ShowsCancelButtonbindable property to SearchBar and SearchHandler classes (default:true) - Implemented iOS-specific handler logic to control UISearchBar cancel button visibility
- Added platform stubs for Android, Windows, and Standard handlers (no-op implementations)
- Created UI test case Issue33008 to demonstrate the feature
Reviewed changes
Copilot reviewed 28 out of 28 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Controls/src/Core/SearchBar/SearchBar.cs | Added ShowsCancelButton property and bindable property definition to SearchBar |
| src/Controls/src/Core/Shell/SearchHandler.cs | Added ShowsCancelButton property and bindable property definition to SearchHandler |
| src/Core/src/Core/ISearchBar.cs | Added ShowsCancelButton property to ISearchBar interface |
| src/Core/src/Handlers/SearchBar/SearchBarHandler.cs | Added mapper entry for ShowsCancelButton property |
| src/Core/src/Handlers/SearchBar/SearchBarHandler.iOS.cs | Implemented MapShowsCancelButton handler for iOS platform |
| src/Core/src/Handlers/SearchBar/SearchBarHandler.Android.cs | Added no-op stub for MapShowsCancelButton (Android doesn't support this) |
| src/Core/src/Handlers/SearchBar/SearchBarHandler.Windows.cs | Added no-op stub for MapShowsCancelButton (Windows doesn't support this) |
| src/Core/src/Handlers/SearchBar/SearchBarHandler.Standard.cs | Added no-op stub for MapShowsCancelButton |
| src/Core/src/Platform/iOS/SearchBarExtensions.cs | Updated ShouldShowCancelButton logic to respect the new property |
| src/Controls/src/Core/Compatibility/Handlers/Shell/iOS/SearchHandlerAppearanceTracker.cs | Added property change handler and UpdateShowsCancelButton method for SearchHandler |
| src/Controls/tests/TestCases.HostApp/Issues/Issue33008.xaml | Created XAML test page demonstrating SearchBar and SearchHandler with different ShowsCancelButton values |
| src/Controls/tests/TestCases.HostApp/Issues/Issue33008.xaml.cs | Code-behind for Issue33008 test with button handler to set text |
| src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue33008.cs | Added UI test for ShowsCancelButton feature |
| src/Core/src/PublicAPI/*/PublicAPI.Unshipped.txt | Updated all platform-specific PublicAPI files with new ISearchBar.ShowsCancelButton property and MapShowsCancelButton handler |
| src/Controls/src/Core/PublicAPI/*/PublicAPI.Unshipped.txt | Updated all platform-specific PublicAPI files with SearchBar and SearchHandler ShowsCancelButton properties |
| { | ||
| App.WaitForElement("TitleLabel"); | ||
| App.Tap("SetTextButton"); | ||
| App.EnterText("Search...", "Test text"); |
There was a problem hiding this comment.
The UI test is using App.EnterText("Search...", "Test text") but there's no element with AutomationId "Search...". Looking at the XAML, the SearchBar elements have AutomationIds "SearchBarDefault", "SearchBarTrue", and "SearchBarFalse", not "Search...". The placeholder text "Search..." is not an AutomationId.
This test will fail at runtime because it's trying to interact with a non-existent element. The test should use one of the actual AutomationIds like "SearchBarDefault".
| App.EnterText("Search...", "Test text"); | |
| App.EnterText("SearchBarDefault", "Test text"); |
| Color CancelButtonColor { get; } | ||
|
|
||
| /// <summary> | ||
| /// Gets a value indicating whether the cancel button should be displayed. |
There was a problem hiding this comment.
[nitpick] The XML documentation for the ISearchBar interface should include information about platform-specific behavior, similar to the documentation on the SearchBar and SearchHandler classes. Currently it only states "Gets a value indicating whether the cancel button should be displayed" which doesn't explain the iOS-specific behavior or the text requirement.
Consider adding: "On iOS, the cancel button appears when the search bar contains text. This property has no effect on Android, Windows, or Tizen platforms."
| /// Gets a value indicating whether the cancel button should be displayed. | |
| /// Gets a value indicating whether the cancel button should be displayed. | |
| /// On iOS, the cancel button appears when the search bar contains text. This property has no effect on Android, Windows, or Tizen platforms. |
| searchBar.ShowsCancelButton && !string.IsNullOrEmpty(searchBar.Text); | ||
|
|
||
| public static void UpdateCancelButton(this UISearchBar uiSearchBar, ISearchBar searchBar) | ||
| { | ||
| // Respect the ShowsCancelButton property - if false, never show the button | ||
| // If true, show it based on whether there's text (iOS standard behavior) | ||
| uiSearchBar.ShowsCancelButton = searchBar.ShouldShowCancelButton(); |
There was a problem hiding this comment.
The comment on lines 124-125 explains the logic, but it's actually documenting two different behaviors:
- If ShowsCancelButton is false, never show the button
- If true, show it based on whether there's text (iOS standard behavior)
However, the actual implementation on line 120 (searchBar.ShowsCancelButton && !string.IsNullOrEmpty(searchBar.Text)) doesn't fully implement "iOS standard behavior." Standard iOS UISearchBar shows the cancel button when focused, regardless of text content.
The current implementation creates a UX inconsistency where the cancel button only appears after typing, which means users can't dismiss the keyboard when the search bar is focused but empty. This could still create a keyboard trap scenario if the user focuses the search bar but hasn't typed anything yet.
| public class Issue33008Shell : Shell | ||
| { | ||
| public Issue33008Shell() | ||
| { | ||
| Items.Add(new Issue33008()); | ||
| } | ||
| } |
There was a problem hiding this comment.
The Issue33008Shell creates a Shell with SearchHandler, but the ContentPage also defines a Shell.SearchHandler in XAML. This creates ambiguity about which SearchHandler is actually being tested.
The typical pattern for Shell-based issue tests is either:
- Use a Shell subclass (Issue33008Shell) and set the SearchHandler in the Shell's constructor
- OR use a ContentPage and define Shell.SearchHandler in XAML
Mixing both approaches is confusing. Consider removing the Issue33008Shell class and just using the ContentPage with the Shell.SearchHandler defined in XAML, or move all SearchHandler configuration to the Shell constructor.
| public class Issue33008Shell : Shell | |
| { | |
| public Issue33008Shell() | |
| { | |
| Items.Add(new Issue33008()); | |
| } | |
| } |
| SearchHandler.Query = "Test text"; | ||
| } |
There was a problem hiding this comment.
The OnSetText method sets SearchHandler.Query = "Test text" but there's no SearchHandler instance accessible in the ContentPage scope. The SearchHandler is defined in the XAML as Shell.SearchHandler, not as a field accessible via this.SearchHandler.
This code will result in a null reference exception at runtime. To fix this, either:
- Add
x:Name="SearchHandler"to the SearchHandler in XAML (already present) and access it via the name - Use
Shell.GetSearchHandler(this)to retrieve the SearchHandler - Store a reference to the SearchHandler in a field during InitializeComponent()
| SearchHandler.Query = "Test text"; | |
| } | |
| var searchHandler = Shell.GetSearchHandler(this); | |
| if (searchHandler != null) | |
| { | |
| searchHandler.Query = "Test text"; | |
| } |
| if (_searchHandler.IsFocused) | ||
| { | ||
| _uiSearchBar.SetShowsCancelButton(_searchHandler.ShowsCancelButton, true); | ||
| } |
There was a problem hiding this comment.
The UpdateShowsCancelButton() method only updates the cancel button visibility if the SearchHandler is currently focused. However, when the property changes while unfocused, the cancel button state won't be updated until the next focus change event.
This could lead to unexpected behavior where changing ShowsCancelButton from true to false while unfocused won't take effect until the user focuses the search bar again. Consider updating the method to handle both focused and unfocused states, or at a minimum, document this behavior limitation.
| if (_searchHandler.IsFocused) | |
| { | |
| _uiSearchBar.SetShowsCancelButton(_searchHandler.ShowsCancelButton, true); | |
| } | |
| _uiSearchBar.SetShowsCancelButton(_searchHandler.ShowsCancelButton, true); |
| if (_searchHandler.ShowsCancelButton) | ||
| { | ||
| _uiSearchBar.SetShowsCancelButton(true, true); | ||
| } |
There was a problem hiding this comment.
When ShowsCancelButton is false, the cancel button is never shown, which means users have no way to dismiss the keyboard on iOS if this property is set to false. The PR description states this fixes a "keyboard trap," but setting ShowsCancelButton="False" would create the exact same keyboard trap.
The iOS implementation should respect the property value while still providing a way to dismiss the keyboard. Consider either:
- Documenting this limitation clearly in the XML documentation
- Providing alternative keyboard dismissal when ShowsCancelButton is false (e.g., tap-outside-to-dismiss)
- Reconsidering whether allowing false is appropriate for iOS
|
|
||
| public static void MapShowsCancelButton(ISearchBarHandler handler, ISearchBar searchBar) | ||
| { | ||
| // ShowsCancelButton is iOS-specific behavior |
There was a problem hiding this comment.
[nitpick] The comment says "ShowsCancelButton is iOS-specific behavior" but this is also applicable to MacCatalyst (which uses UISearchBar). The comment should be updated to reflect that this is iOS/MacCatalyst-specific behavior, or more accurately, that Windows doesn't have an equivalent feature in AutoSuggestBox.
| // ShowsCancelButton is iOS-specific behavior | |
| // ShowsCancelButton is an iOS and MacCatalyst-specific behavior (UISearchBar). | |
| // Windows' AutoSuggestBox does not have an equivalent feature. |
| /// <summary> | ||
| /// Gets or sets a value that indicates whether the cancel button is shown. | ||
| /// </summary> | ||
| /// <remarks> | ||
| /// On iOS, the cancel button appears when the search bar is focused and allows users to dismiss the keyboard. | ||
| /// Default value is <c>true</c>. | ||
| /// </remarks> | ||
| public bool ShowsCancelButton | ||
| { | ||
| get { return (bool)GetValue(ShowsCancelButtonProperty); } | ||
| set { SetValue(ShowsCancelButtonProperty, value); } | ||
| } |
There was a problem hiding this comment.
The XML documentation states "On iOS, the cancel button appears when the search bar is focused and allows users to dismiss the keyboard." However, based on the implementation in SearchBarExtensions.ShouldShowCancelButton(), the cancel button only appears when BOTH conditions are true: the property is true AND there is text in the search bar.
The documentation should be updated to accurately reflect this behavior: "On iOS, the cancel button appears when the search bar is focused and contains text, allowing users to dismiss the keyboard. Default value is true."
| /// <summary> | ||
| /// Gets or sets a value that indicates whether the cancel button is shown. | ||
| /// </summary> | ||
| /// <remarks> | ||
| /// On iOS, the cancel button appears when the search bar is focused and allows users to dismiss the keyboard. | ||
| /// Default value is <c>true</c>. | ||
| /// </remarks> | ||
| public bool ShowsCancelButton | ||
| { | ||
| get { return (bool)GetValue(ShowsCancelButtonProperty); } | ||
| set { SetValue(ShowsCancelButtonProperty, value); } | ||
| } |
There was a problem hiding this comment.
The XML documentation states "On iOS, the cancel button appears when the search bar is focused and allows users to dismiss the keyboard." However, based on the implementation in SearchBarExtensions.ShouldShowCancelButton(), the cancel button only appears when BOTH conditions are true: the property is true AND there is text in the search bar.
The documentation should be updated to accurately reflect this behavior: "On iOS, the cancel button appears when the search bar is focused and contains text, allowing users to dismiss the keyboard. Default value is true."
1 participant
[Issue-Resolver] Fix #33008 - Add ShowsCancelButton property to SearchBar and SearchHandler
Fixes #33008
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!
Summary
This PR fixes the keyboard trap issue on iOS where the SearchHandler cancel button was hardcoded to hidden, preventing users from dismissing the keyboard. The fix adds a new
ShowsCancelButtonproperty to bothSearchBarandSearchHandler, giving developers full control while fixing the reported bug by defaulting totrue.Quick verification:
true(expected behavior)falseif providing alternative dismissal📋 Click to expand full PR details
Root Cause
The iOS
SearchHandlerAppearanceTrackerhad hardcoded behavior on line 328-330:And on line 43, during initialization:
This created a keyboard trap where:
Additionally, there was no API for developers to control cancel button visibility on either
SearchBarorSearchHandler.Solution
Added
ShowsCancelButtonproperty to bothSearchBarandSearchHandlerwith a default value oftrue.1. SearchHandler Implementation
File:
src/Controls/src/Core/Shell/SearchHandler.csShowsCancelButtonPropertybindable property (default:true)ShowsCancelButtonproperty with XML documentationFile:
src/Controls/src/Core/Compatibility/Handlers/Shell/iOS/SearchHandlerAppearanceTracker.csOnEditingStartedto checkShowsCancelButtonproperty before showing buttonOnEditingEndedto checkShowsCancelButtonproperty before hiding buttonUpdateShowsCancelButton()method for dynamic property changesShowsCancelButtonPropertyChanges made:
2. SearchBar Implementation
File:
src/Controls/src/Core/SearchBar/SearchBar.csShowsCancelButtonPropertybindable property (default:true)ShowsCancelButtonproperty with XML documentationFile:
src/Core/src/Core/ISearchBar.csShowsCancelButtonproperty to interfaceFile:
src/Core/src/Handlers/SearchBar/SearchBarHandler.cs[nameof(ISearchBar.ShowsCancelButton)] = MapShowsCancelButtonPlatform-specific handlers:
SearchBarHandler.iOS.cs): Full implementationSearchBarHandler.Android.cs): No-op stub (SearchView doesn't have equivalent)SearchBarHandler.Windows.cs): No-op stub (AutoSuggestBox doesn't have equivalent)SearchBarHandler.Standard.cs): No-op stubFile:
src/Core/src/Platform/iOS/SearchBarExtensions.csShouldShowCancelButton()extension method:How It Works
iOS Standard Behavior
The cancel button visibility follows iOS UISearchBar standard behavior:
ShowsCancelButton == trueAND search bar has textShowsCancelButton == falseOR search bar is emptySearchHandler
SearchBar
Testing
Reproduction of Original Issue
Before fix - Keyboard trap:
After fix - Cancel button available:
Device Tests Added
File:
src/Controls/tests/DeviceTests/Elements/SearchBar/SearchBarTests.cs5 comprehensive device tests verify the property behavior:
✅ ShowsCancelButtonDefaultsToTrue
true✅ ShowsCancelButtonTrueShowsCancelButton
ShowsCancelButton=trueUISearchBar.ShowsCancelButtonreturnstrue✅ ShowsCancelButtonFalseHidesCancelButton
ShowsCancelButton=falseUISearchBar.ShowsCancelButtonreturnsfalse✅ ShowsCancelButtonNoTextNeverShowsCancelButton
ShowsCancelButton=truebut no text✅ ShowsCancelButtonCanBeToggledDynamically
Helper method added:
GetPlatformShowsCancelButton()inSearchBarTests.iOS.csUISearchBar.ShowsCancelButtonpropertyEdge Cases Tested
truefalsetruePlatforms Tested
Platform Support
UISearchBar.ShowsCancelButtonSearchViewdoesn't have equivalent cancel buttonAutoSuggestBoxdoesn't have equivalent cancel buttonAPI consistency: Property exists on all platforms for consistent API surface, but only affects behavior on iOS/MacCatalyst.
Breaking Changes
None
true(matches expected iOS behavior)true)falseFiles Changed
SearchHandler: 2 files
src/Controls/src/Core/Shell/SearchHandler.cssrc/Controls/src/Core/Compatibility/Handlers/Shell/iOS/SearchHandlerAppearanceTracker.csSearchBar: 9 files
src/Controls/src/Core/SearchBar/SearchBar.cssrc/Core/src/Core/ISearchBar.cssrc/Core/src/Handlers/SearchBar/SearchBarHandler.cssrc/Core/src/Handlers/SearchBar/SearchBarHandler.iOS.cssrc/Core/src/Handlers/SearchBar/SearchBarHandler.Android.cssrc/Core/src/Handlers/SearchBar/SearchBarHandler.Windows.cssrc/Core/src/Handlers/SearchBar/SearchBarHandler.Standard.cssrc/Core/src/Platform/iOS/SearchBarExtensions.csDevice Tests: 2 files
src/Controls/tests/DeviceTests/Elements/SearchBar/SearchBarTests.cs(5 tests added)src/Controls/tests/DeviceTests/Elements/SearchBar/SearchBarTests.iOS.cs(helper method)PublicAPI: 15 files
PublicAPI.Unshipped.txtfiles updated (Core: 8 platforms, Controls: 7 platforms)Total: 28 files modified
Implementation Details
Property Definition
Both
SearchBarandSearchHandleruse the same pattern:iOS Handler Logic
The iOS handler respects the property in two places:
OnEditingStarted):OnEditingEnded):Checklist
ShowsCancelButton = false)true)