mozilla-mobile / firefox-ios

Firefox for iOS
Mozilla Public License 2.0
12.2k stars 2.92k forks source link

Remove arrow from search engine selection popover on iPad and investigate copying Safari behaviour #22747

Closed data-sync-user closed 3 hours ago

data-sync-user commented 3 hours ago

This ticket is for the Unified Search project.

You must enable both the toolbar refactor and the unified_search feature flag to work on this task. (You can also toggle them both on from the Settings > Debug Menu > Feature Flags).

Problem 1: We need to remove the iPad popup arrow on the search engine selection popup

Currently on iPad, tapping the search engine icon shows a popover with an arrow like so:

!Screenshot 2024-10-24 at 12.04.38 PM.png|width=1382,height=1048,alt="Screenshot 2024-10-24 at 12.04.38 PM.png"!

We want to remove the arrow from our final designs and have the popover display beneath the iPad toolbar like so:

!Screenshot 2024-10-24 at 12.02.43 PM.png|width=345,height=209,alt="Screenshot 2024-10-24 at 12.02.43 PM.png"!

Problem 2: Can we make the popup go under the keyboard like Safari for small iPad screens?

On small tablets like the ﹍iPad Mini 6th gen﹍, we also have a second problem with the keyboard. The keyboard shrinks the popup and moves it beside the search engine icon.

!Screenshot 2024-10-24 at 12.08.34 PM.png|width=1338,height=1004,alt="Screenshot 2024-10-24 at 12.08.34 PM.png"!

The designer says this is acceptable, but he wanted us to investigate whether we can copy what Safari does. Safari is able to have the popup go under the keyboard. Then the user can tap on the popup to close the keyboard and make a selection.

Example of Safari:

!Safari behind keyboard example.png|width=1382,height=1048,alt="Safari behind keyboard example.png"!

Code: Where to make the change…

Please refer to the file BrowserCoordinator.swift for the current presentation logic of this bottom sheet (iPhone) and popover (iPad). The presentation style is currently configured in:

func showSearchEngineSelection(forSourceView sourceView: UIView) {
        guard !childCoordinators.contains(where: { $0 is SearchEngineSelectionCoordinator }) else { return }

        let navigationController = DismissableNavigationViewController()
        if navigationController.shouldUseiPadSetup() {
            navigationController.modalPresentationStyle = .popover
            navigationController.preferredContentSize = UX.searchEnginePopoverSize
            navigationController.popoverPresentationController?.sourceView = sourceView
            navigationController.popoverPresentationController?.canOverlapSourceViewRect = false
        } else {
            navigationController.modalPresentationStyle = .pageSheet
            navigationController.sheetPresentationController?.detents = [.medium(), .large()]
            navigationController.sheetPresentationController?.prefersGrabberVisible = true
        }

        let coordinator = DefaultSearchEngineSelectionCoordinator(
            router: DefaultRouter(navigationController: navigationController),
            windowUUID: tabManager.windowUUID
        )

        coordinator.parentCoordinator = self
        coordinator.navigationHandler = self
        add(child: coordinator)
        coordinator.start()

        present(navigationController)
    }

As well, there is a SearchEngineSelectionCoordinator if needed. The SearchEngineSelectionViewController (which is the view inside the popup/bottom sheet) is currently just a stub placeholder which will be filled later.

┆Issue is synchronized with this Jira Task

data-sync-user commented 3 hours ago

➤ ih-codes commented:

After chatting with Verdi, this is actually no longer a concern. We don’t want to emulate Safari because closing the keyboard should revert the search ( https://mozilla-hub.atlassian.net/browse/FXIOS-10264 ), and we can simply use the default iOS arrow to point to the source view and let the view layout as best it can on small screens (iPad mini in landscape with the keyboard displayed).