Fix Search bars on iPadOS 26#5703
Conversation
tonisevener
requested review from
a team and
l-olson1214
and removed request for
a team
| isEnabled = "YES"> | ||
| </EnvironmentVariable> | ||
| </EnvironmentVariables> | ||
| <LocationScenarioReference |
There was a problem hiding this comment.
Were the edits in this file intended to be here?
There was a problem hiding this comment.
Nope, thanks for catching!
There was a problem hiding this comment.
Pull request overview
This PR addresses iPadOS 26 search bar UX issues by introducing an iPad-specific search bar customizer and refactoring the legacy SearchViewController into separate view controllers tailored to each presentation context (tab root vs. embedded search results vs. minimal pushed search).
Changes:
- Refactor search flow: replace
SearchViewControllerwithSearchTabViewController,SearchResultsViewController(assearchResultsController), andSearchMinimalViewController. - Add
SearchBarIPadCustomizerand wire it into search bars that are affected on iPad (Search, Saved, Places, Reading List Detail). - Update History SwiftUI view model/view to support bottom safe-area padding.
Reviewed changes
Copilot reviewed 27 out of 85 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| Wikipedia/Localizations/en.lproj/Localizable.strings | Remove deprecated reading-list search placeholder key (en). |
| Wikipedia/Localizations/qqq.lproj/Localizable.strings | Remove deprecated reading-list search placeholder key (qqq). |
| Wikipedia/Code/WMFAppViewController.m | Swap search tab root VC from SearchViewController to SearchTabViewController; update navigation to pushed minimal search. |
| Wikipedia/Code/WMFAppViewController+Extensions.swift | Update tab-selection logging type checks to SearchTabViewController. |
| Wikipedia/Code/SinglePageWebViewController.swift | Use SearchMinimalViewController when pushing search. |
| Wikipedia/Code/SearchTabViewController.swift | New Search tab root controller (history background + system search controller). |
| Wikipedia/Code/SearchResultsViewController.swift | New “searchResultsController-only” container coordinating recent searches, languages bar, and results list + iPad workarounds. |
| Wikipedia/Code/SearchResultsListViewController.swift | Extract the old collection-based results list into its own VC. |
| Wikipedia/Code/SearchMinimalViewController.swift | New lightweight pushed search container. |
| Wikipedia/Code/SearchBarIPadCustomizer.swift | New iPadOS 26 search bar customization helper (custom back + clear). |
| Wikipedia/Code/SavedViewController.swift | Apply iPad customizer to Saved search bar and keep custom clear button in sync. |
| Wikipedia/Code/ReadingListDetailViewController.swift | Remove SearchBarExtendedViewController usage; switch to system search + iPad customizer. |
| Wikipedia/Code/PlacesViewController.swift | Apply iPad customizer to Places search bar and keep custom clear button in sync. |
| Wikipedia/Code/NSError+WMFExtensions.h / .m | Add wmf_isCancelledError helper. |
| Wikipedia/Code/InsertLinkViewController.swift | Use SearchResultsViewController as the searchResultsController for link insertion search. |
| Wikipedia/Code/HintController.swift | Update keyboard-hiding special case to new search results list VC type. |
| Wikipedia/Code/ExploreViewController.swift | Use SearchResultsViewController as embedded searchResultsController and wire up navigation callbacks. |
| Wikipedia/Code/EditLinkViewController.swift | Use SearchMinimalViewController for edit-link article search. |
| Wikipedia/Code/ArticleViewController.swift | Replace embedded search results controller with SearchResultsViewController; adjust iPadOS 26 tab bar behavior. |
| Wikipedia.xcodeproj/xcshareddata/xcschemes/Wikipedia.xcscheme | Enable location simulation + set a default location scenario. |
| Wikipedia.xcodeproj/project.pbxproj | Add new search-related files; remove some old search-related entries (but still contains stale SearchViewController.swift references). |
| WMFComponents/Sources/WMFComponents/Components/History/WMFHistoryViewModel.swift | Add bottomPadding to support safe-area layout. |
| WMFComponents/Sources/WMFComponents/Components/History/WMFHistoryView.swift | Use content margins for top/bottom padding instead of header padding. |
| searchBarIPadCustomizer.onClearTapped = { [weak self] searchController in | ||
| guard let self, | ||
| let searchBar = searchController?.searchBar else { return } | ||
| self.searchBar(searchBar, textDidChange: "") |
There was a problem hiding this comment.
Optional: textDidChange taking "" feels odd. Can we make it take nil, have a different name, etc?
There was a problem hiding this comment.
Yeah I agree it's weird - for some reason programmatically clearing the search bar text (which I do upon clear button tap) is not triggering the UISearchBarDelegate textDidChange method, which is a system method that takes a non-optional search term. So I'm calling it manually here, but it does feel like I must be doing something wrong. Open any ideas here!
| self?.readingListEntryCollectionViewController.updateSearchString("") | ||
| } | ||
| return customizer | ||
| }() | ||
|
|
There was a problem hiding this comment.
SearchBarIPadCustomizer’s custom back button only deactivates the UISearchController; in this VC no onBackTapped/onWillDismiss handler is set to clear the text and reset the filtering state. Previously the cancel/resign flow explicitly cleared the search text and called updateSearchString(""), so on iPadOS 26 the new back button can leave the reading list permanently filtered after exiting the focused search state. Consider wiring searchiPadCustomizer.onBackTapped (and/or onWillDismiss) to clear the search bar text and reset readingListEntryCollectionViewController.updateSearchString("").
| self?.readingListEntryCollectionViewController.updateSearchString("") | |
| } | |
| return customizer | |
| }() | |
| self?.clearSearch() | |
| } | |
| customizer.onBackTapped = { [weak self] _ in | |
| self?.clearSearch() | |
| } | |
| customizer.onWillDismiss = { [weak self] _ in | |
| self?.clearSearch() | |
| } | |
| return customizer | |
| }() | |
| private func clearSearch() { | |
| navigationItem.searchController?.searchBar.text = "" | |
| readingListEntryCollectionViewController.updateSearchString("") | |
| } | |
| fetcher.fetchArticles(forSearchTerm: searchTerm, siteURL: siteURL, resultLimit: WMFMaxSearchResultLimit, failure: { error in | ||
| failure(error, .prefix) | ||
| }, success: { [weak self] results in | ||
| guard let self else { return } | ||
| success(results, .prefix) | ||
| guard let resultsArray = results.results, resultsArray.count < 12 else { return } | ||
| self.fetcher.fetchArticles(forSearchTerm: searchTerm, siteURL: siteURL, resultLimit: WMFMaxSearchResultLimit, fullTextSearch: true, appendToPreviousResults: results, failure: { error in | ||
| failure(error, .full) | ||
| }, success: { [weak self] fullTextResults in | ||
| guard self != nil else { return } | ||
| success(fullTextResults, .full) | ||
| }) |
There was a problem hiding this comment.
In the prefix + full-text search flow, if the full-text request fails you call failure(error, .full), which clears resultsViewController.results and sets an empty state. That will wipe already-successful prefix results and show “no results / no internet” even though the user still has valid prefix matches. Consider keeping the existing prefix results on .full failure (e.g., only log the error, or only apply the failure UI when there are currently no results).
| @objc private func userDidSaveOrUnsaveArticle(_ notification: Notification) { | ||
| guard let article = notification.object as? WMFArticle else { return } | ||
| let context: [String: Any] = [ReadingListHintController.ContextArticleKey: article] | ||
| hintController?.toggle(presenter: self, context: context, theme: theme) |
There was a problem hiding this comment.
userDidSaveOrUnsaveArticle(_:) always toggles the reading-list hint with presenter: self. When a user saves/unsaves from a VC pushed on top of the Search tab’s navigation stack (e.g. an article opened from history/search), SearchTabViewController won’t be the visible presenter, so the hint can fail to display or appear behind the top VC. Consider presenting from the currently visible view controller (e.g. navigationController?.topViewController as? (UIViewController & HintPresenting)) similar to the previous visibleHintPresentingViewController logic.
| hintController?.toggle(presenter: self, context: context, theme: theme) | |
| let presenter = (navigationController?.topViewController as? (UIViewController & HintPresenting)) ?? self | |
| hintController?.toggle(presenter: presenter, context: context, theme: theme) |
Phabricator: https://phabricator.wikimedia.org/T418174
Notes
This PR adds a custom back button to the iPadOS 26 search bar, to allow users to easily exit out of the search focused state. I decided to refactor SearchViewController to get the search bars to act a bit more predictably. Now instead of a single SearchViewController class that must handle 3 different presentation contexts, I have split it up into multiple classes that handle each context.
Old
Single SearchViewController class that was used:
- Embedded as the 5th tab in the tab bar controller
- Embedded as the search results controller in Explore and Article's navigationItem.searchController.searchResultsController
- Pushed on or presented individually (SinglePageWebViewController, editor Insert Link and Edit Link flows)
New
SearchResultsViewController
SearchTabViewController
SearchMinimalViewController
ExploreViewController and ArticleViewController now set SearchResultsViewController as navigationItem.searchController.searchResultsController.
SearchBarIPadCustomizer
This is the class adds the customization for iPadOS 26 search bars. It does not affect iPhones or older iPadOSes, it just serves as a passthrough for them.
SearchResultsViewController uses SearchBarIPadCustomizer to apply the fix to the search bars. I also had to to apply the fix to Places, Saved, and Reading List Detail search bars. Those views do not actually search SearchResultsViewController so they call SearchBarIPadCustomizer directly.
Remaining
I'd like to closely go back over the logging for these to confirm they still work. The hint controllers do not display when saving an article from search results, nor does long press > open in new tab open in a new tab. I will apply these fixes in a followup PR. It may be best to wait on fixing hints until toast rewrites are complete.
Disclaimer
Heavy use of Copilot on this one.
Test Steps
Screenshots/Videos