Bug 408290

Summary: Placing focus in the searchbar makes Esc unable to dismiss the applet
Product: [Plasma] plasma-nm Reporter: Kishore Gopalakrishnan <kishore96>
Component: appletAssignee: Jan Grulich <jgrulich>
Status: RESOLVED FIXED    
Severity: minor CC: kde, nate
Priority: NOR Keywords: usability
Version: 5.15.90   
Target Milestone: ---   
Platform: Arch Linux   
OS: Linux   
Latest Commit: https://commits.kde.org/plasma-nm/5b45ac0edc140518769eda0b7a7ff59b936a5027 Version Fixed In: 5.16.1
Sentry Crash Report:

Description Kishore Gopalakrishnan 2019-06-04 13:04:11 UTC 
SUMMARY
Once we place focus in the searchbar, 'Esc' is unable to dismiss the applet no matter how many times we press it. The applet is able to be dismissed using 'Esc' after interacting with other elements in the applet.

STEPS TO REPRODUCE
1. Open the Networks popup in the systray.
2. Click the search icon above the channels list.
3. Click in the search area to place focus there.
4. Press Esc. Searchbar disappears.
5. Press Esc again. Note that no matter how many times you press Esc, the popup does not close.
6. If you have an active wifi network, click on it. Click on the 'details tab'.
7. Now note that Esc works to close the popup.

OBSERVED RESULT
Esc doesn't allow the applet to be dismissed after step 5 without further manual interaction with the applet.

EXPECTED RESULT
Esc should allow the applet to be dismissed after step 5. I.e. first press of Esc should remove focus from the searchbar, and the second press should close the applet.

SOFTWARE/OS VERSIONS
Linux distro: Arch Linux
KDE Plasma Version: 5.15.90
KDE Frameworks Version: 5.58.0
Qt Version: 5.13.0

ADDITIONAL INFORMATION
It looks like the serachbar isn't giving up focus correctly when one presses Esc.
Comment 1 Nate Graham 2019-06-05 14:55:35 UTC 
Probably caused by https://cgit.kde.org/plasma-nm.git/tree/applet/contents/ui/Toolbar.qml#n154

Looks like a good patch opportunity!
Comment 2 Kai Uwe Broulik 2019-06-06 07:16:16 UTC 
Probably needs to be changed to:

Keys.onEscapePressed: {
    if (searchToggleButton.checked) {
        searchToggleButton.checked = false;
    } else {
        event.accepted = false;
    }
}
Comment 3 Kishore Gopalakrishnan 2019-06-06 14:04:34 UTC 
(In reply to Kai Uwe Broulik from comment #2)
> Probably needs to be changed to:
> 
> Keys.onEscapePressed: {
>     if (searchToggleButton.checked) {
>         searchToggleButton.checked = false;
>     } else {
>         event.accepted = false;
>     }
> }

This seems to work. Should I submit the patch?

Note: I found another similar bug. If we click on some elements in the networks list in a particular order, the searchbox refuses to accept text that we type until we manually place the focus in the searchbox. I'll try to see how that can be fixed.
Comment 4 Nate Graham 2019-06-06 14:05:41 UTC 
(In reply to Kishore Gopalakrishnan from comment #3)
> (In reply to Kai Uwe Broulik from comment #2)
> > Probably needs to be changed to:
> > 
> > Keys.onEscapePressed: {
> >     if (searchToggleButton.checked) {
> >         searchToggleButton.checked = false;
> >     } else {
> >         event.accepted = false;
> >     }
> > }
> 
> This seems to work. Should I submit the patch?
Yes please!

> Note: I found another similar bug. If we click on some elements in the
> networks list in a particular order, the searchbox refuses to accept text
> that we type until we manually place the focus in the searchbox. I'll try to
> see how that can be fixed.
If you do, feel free to submit a patch for that, too.
Comment 5 Kishore Gopalakrishnan 2019-06-06 14:44:47 UTC 
(In reply to Nate Graham from comment #4)
> (In reply to Kishore Gopalakrishnan from comment #3)
> > (In reply to Kai Uwe Broulik from comment #2)
> > > Probably needs to be changed to:
> > > 
> > > Keys.onEscapePressed: {
> > >     if (searchToggleButton.checked) {
> > >         searchToggleButton.checked = false;
> > >     } else {
> > >         event.accepted = false;
> > >     }
> > > }
> > 
> > This seems to work. Should I submit the patch?
> Yes please!
> 
> > Note: I found another similar bug. If we click on some elements in the
> > networks list in a particular order, the searchbox refuses to accept text
> > that we type until we manually place the focus in the searchbox. I'll try to
> > see how that can be fixed.
> If you do, feel free to submit a patch for that, too.

I've submitted it here. https://phabricator.kde.org/D21620
Comment 6 Nate Graham 2019-06-07 14:10:48 UTC 
Git commit 5b45ac0edc140518769eda0b7a7ff59b936a5027 by Nate Graham, on behalf of Kishore Gopalakrishnan.
Committed on 07/06/2019 at 14:10.
Pushed by ngraham into branch 'Plasma/5.16'.

Handle Esc properly when focus is in searchbox

Summary:

If we place the focus in the searchbar, pressing Esc once dismisses the searchbar. However, pressing Esc after this does not dismiss the popup unless one manually intereacts with some other element in the popup. This patch fixes the issue.

Test Plan:
1. Open the Networks popup in the systray.
2. Click the search icon above the channels list.
3. Click in the search area to place focus there.
4. Press Esc. Searchbar disappears.
5. Press Esc again. Check that the popup closes.

Reviewers: #plasma, jgrulich, ngraham

Reviewed By: jgrulich, ngraham

Subscribers: ngraham, plasma-devel

Tags: #plasma

Differential Revision: https://phabricator.kde.org/D21620

M  +8    -1    applet/contents/ui/Toolbar.qml

https://commits.kde.org/plasma-nm/5b45ac0edc140518769eda0b7a7ff59b936a5027