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

[SearchView] When using android:enableOnBackInvokedCallback="true" , the SearchView is collapsed together with the keyboard, instead of just the keyboard #4185

Open
AndroidDeveloperLB opened this issue May 21, 2024 · 10 comments

Comments

@AndroidDeveloperLB
Copy link

AndroidDeveloperLB commented May 21, 2024

Description: Full description of issue here
I have a toolbar with a search menu item in it.
I also use setOnActionExpandListener on it to handle some stuff, related to predictive back gesture .
As such, I enable the predictive back gesture in the manifest:

  android:enableOnBackInvokedCallback="true" 

All works fine everywhere else, except here. When I put a focus on the SearchView and I enter something, and then press back key, it closes both the keyboard AND the Search View, making it impossible to go back to it and change it, and of course to see the results in case the search was performed.
When the above is set to false, it works fine.

Expected behavior: Screenshots and/or description of expected behavior
Should close only the keyboard first, and only if I press back key again, it should close the SearchView.

Source code: The code snippet which is causing this issue

    override fun onCreateOptionsMenu(menu: Menu): Boolean {
        // Inflate the menu; this adds items to the action bar if it is present.
        menuInflater.inflate(R.menu.menu_main, menu)
        menu.findItem(R.id.menuItem_search)
            .setOnActionExpandListener(object : OnActionExpandListener {
                override fun onMenuItemActionExpand(item: MenuItem): Boolean {
                    Log.d("AppLog", "onMenuItemActionExpand")
                    return true
                }

                override fun onMenuItemActionCollapse(item: MenuItem): Boolean {
                    Log.d("AppLog", "onMenuItemActionCollapse")
                    return true
                }
            })
        return true
    }
<menu xmlns:android="http://schemas.android.com/apk/res/android"
    xmlns:app="http://schemas.android.com/apk/res-auto"
    xmlns:tools="http://schemas.android.com/tools"
    tools:context="com.lb.myapplication.MainActivity">

    <item
        android:id="@+id/menuItem_search"
        android:icon="@android:drawable/ic_search_category_default"
        android:title="search"
        app:actionViewClass="androidx.appcompat.widget.SearchView"
        app:showAsAction="always|collapseActionView" />

    <item
        android:id="@+id/action_settings" android:orderInCategory="100"
        android:title="@string/action_settings" app:showAsAction="never" />
</menu>

Minimal sample app repro: Please consider attaching a minimal sample app that reproduces the issue. This will help narrow down the conditions required for reproducing the issue, and it will speed up the bug fix process. You may attach a zip file of the sample app or link to a GitHub repo that contains the sample app.

My Application.zip

Video:

studio64_2024-05-21_14-21-38.zip

Android API version: Android API version here
34

Material Library version: Material Android Library version you are using here (e.g., 1.1.0-alpha07)

implementation 'com.google.android.material:material:1.13.0-alpha02'

Device: Device on which the bug was encountered here
Pixel 6

I can see a similar behavior on the sample of the repository, but here it's not in the toolbar at all, and it's also inconsistent. Watch:

scrcpy_2024-05-21_14-31-57.zip

I've also reported about this here but nobody has fixed it yet, and even my sample video and sample project were deleted by the forum:
https://issuetracker.google.com/issues/326681254

@AndroidDeveloperLB
Copy link
Author

AndroidDeveloperLB commented May 21, 2024

Please update the sample with normal SearchView which is a part of a normal toolbar, and not just the fancy one.
This can help to find such issues, and also handle them no matter what the app was chosen to use

@dsn5ft
Copy link
Contributor

dsn5ft commented May 23, 2024

@AndroidDeveloperLB the SearchView embedded within a Toolbar is a somewhat outdated pattern, and it's not part of the Material spec anymore. Additionally, androidx.appcompat.widget.SearchView is not part of the Material library so we have no control over it.

Is there a reason you can't use the newer Material SearchBar + SearchView? Or if you don't want to use SearchBar, you could use a regular Toolbar with search icon and on click show the SearchView. That should be a better user experience because it gives more space for the user to enter search text, and also you can show search history / suggestions in the SearchView.

@dsn5ft dsn5ft closed this as completed May 23, 2024
@AndroidDeveloperLB
Copy link
Author

@dsn5ft

  1. So how can I have a search in a toolbar? I now have a toolbar with multiple action items. Where would I put something else? How would I migrate to something else without removing any functionality that users have?

  2. Why do you think the newer one is better, and you mark it as "completed" , if I've shown you that it occurs even here on this sample of this repository?

  3. What is the better user experience? It's still a search box. Users put text inside and that's it. What extra functionality is there here? There is no more space at all. It's still a single line to put text.

Please explain.

@dsn5ft
Copy link
Contributor

dsn5ft commented May 23, 2024

  1. So how can I have a search in a toolbar? I now have a toolbar with multiple action items. Where would I put something else? How would I migrate to something else without removing any functionality that users have?

Well there are two potential options:

  1. You could still have the search icon in the toolbar, but instead of using app:actionViewClass="androidx.appcompat.widget.SearchView" you could show a Material SearchView on click of the search icon. That would not remove any of your current functionality.

  2. Use a Material SearchBar instead of a toolbar and put your additional action items in the SearchBar (examples). Clicking on the SearchBar itself would initiate the search, and the user would still have the ability to click on the additional action items.

image

Why do you think the newer one is better, and you mark it as "completed" , if I've shown you that it occurs even here on this sample of this repository?

As I mentioned, androidx.appcompat.widget.SearchView is not part of the Material library / repository. So this is not the right place to file the bug. I see you already filed one for AppCompat so I'd suggest continuing to follow up on that.

  1. What is the better user experience? It's still a search box. Users put text inside and that's it. What extra functionality is there here? There is no more space at all. It's still a single line to put text.

I didn't realize that the AppCompat SearchView also hides the additional action items, so I guess the space for searching is roughly the same.

One other potential advantage of the Material SearchView is that it allows you to put action items within the expanded search area, e.g. a microphone option for voice search, or any other actions that could be useful during search (like filters):

image

@AndroidDeveloperLB
Copy link
Author

AndroidDeveloperLB commented May 23, 2024

  1. Screenshots show it's more space for not more features. It looks nice though. I will still be able to perform all operations as I can do now on the Toolbar?

  2. Please fix the bug here. The one of the issue tracker wasn't fixed for a long time.
    Why did you close it here ? The bug exists on the sample itself, as I've shown.
    Also, please show each of the ways to implement in the sample, after you fix this bug.

Right now this bug exists on all types of search UIs, whether by appcompatx or material.

@dsn5ft
Copy link
Contributor

dsn5ft commented May 23, 2024

  1. Screenshots show it's more space for not more features. It looks nice though. I will still be able to perform all operations as I can do now on the Toolbar?

The Material SearchBar extends from Toolbar, so you should be able to perform the same operations.

  1. Please fix the bug here. The one of the issue tracker wasn't fixed for a long time.

This whole time I actually thought you were talking about Gesture Navigation, since you mentioned predictive back, which does not have the issue:

Search.Bar.Keyboard.mov

I only just realized that you are using the 3-Button Navigation mode, so I now see the keyboard issue for Material SearchView when using that mode. Reopening so we can investigate that.

@dsn5ft dsn5ft reopened this May 23, 2024
@AndroidDeveloperLB
Copy link
Author

AndroidDeveloperLB commented May 23, 2024

Why would you think about gestures when I clearly showed you a video with 3 buttons navigation and I never mentioned gestures, and I also wrote "press back key"?

Also, the same issue occurs with gestures: When I make the back-gesture, instead of closing the keyboard and that's it, it closes both the keyboard and the search box completely.

Try it for yourself. Change android:enableOnBackInvokedCallback to false and compare with when it's true. You will see the behavior is different, even though it's just a back key/gesture. It should always have the same behavior : just close the keyboard alone first.

Please fix it for all types of navigation, and for all types of toolbars and search components. It doesn't make sense.

scrcpy_2024-05-23_22-08-08.zip

@dsn5ft
Copy link
Contributor

dsn5ft commented May 24, 2024

Why would you think about gestures when I clearly showed you a video with 3 buttons navigation and I never mentioned gestures, and I also wrote "press back key"?

You started with "I also use setOnActionExpandListener on it to handle some stuff, related to predictive back gesture" and "I enable the predictive back gesture in the manifest", so that made it sound like you were using gesture navigation.

Additionally, as you can see below even when using gesture navigation there is still a back key when the keyboard is shown:

image

Regardless, it was a simple misunderstanding. Please be respectful. We will look into this issue.

@AndroidDeveloperLB
Copy link
Author

AndroidDeveloperLB commented May 24, 2024

@dsn5ft That's not a back key. That's a hide-keyboard key. Gesture navigation has no back key in the navigation bar. It has back gesture on the sides, and back-gesture always functions exactly as the back key, doesn't matter where it came from.

If you use a keyboard or the emulator's back key or use adb command, you will see that doing so (meaning back key and back gesture, as I've written already) has the same bug : It closes the keyboard and the search itself.

As for misunderstanding, again, just because I've implemented something doesn't mean I'm talking about a scenario that is related to it. I specifically mentioned pressing a key, I've shown 2 videos about it, and I've shown a link to the same issue on the issue tracker.

Please fix this bug on all cases. All navigation methods should be consistent in their behavior. All UI components should be consistent in their behavior : searchbar/toolbar/editText of all kinds .

@AndroidDeveloperLB
Copy link
Author

AndroidDeveloperLB commented May 29, 2024

@dsn5ft Actually, even on the main screen of the sample there is a problem with searching, as the keyboard doesn't always show up when reaching the search, and the back key/gesture might close the keyboard together with the Activity (not sure in which scenarios, but it happened).

I personally don't like any of the search/toolbar UIs that are on the sample. I prefer the original one we had, that has a title that's properly aligned to the language, with action buttons on the side.

A whole row of search makes sense only if the entire screen right now is for searching and there is only one main UI of the app.

AndroidDeveloperLB referenced this issue Jul 9, 2024
…in onAttachedToWindow() and remove modal for accessibility state in onDetachedFromWindow()

Fixes an issue where nothing is focusable in TalkBack after removing SearchView, due to the modal for accessibility behavior

Resolves #4176

PiperOrigin-RevId: 644038017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants