Differential D24935

Fix search field focus, both initial and on-demand
ClosedPublic
Actions

Authored by ngraham on Oct 24 2019, 9:17 PM.

Details

Reviewers
apol
davidedmundson
Group Reviewers
Discover Software Store
Commits
R134:5d1b3718efb8: Fix search field focus, both initial and on-demand
Summary

Right now the search field doesn't get focus initially, or when the user types on a page.
This is because the conditional loader that conditionally loads the GlobalDrawer's
toolbar containing the search field isn't able to pass on focus or signals to its content
properly. I tried various methods of fixing this by adding focus: true to all the focus
stopes in the chain, but was unsuccessful. So this patch implements the conditional
loading of the top content differently: instead the top content is always loaded, but
conditionally discarded when in non-widescreen mode. This fixes the aforementioned issues.

However it may not be the correct way to fix the problem; if not, assistance would be
appreciated.

BUG: 413407
FIXED-IN: 5.18.0

Test Plan
  • Search field has focus when Discover is launched
  • Search field gains focus and accept text when typing anywhere
  • When the window is made mobiley, the sidebar's toolbar disappears
  • When a mobiley window is made desktoppy, the sidebar;s toolbar re-appears

Diff Detail

Repository
R134 Discover Software Store
Branch
fix-searchfield-focus (branched from master)
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 18145
Build 18163: arc lint + arc unit
ngraham created this revision.Oct 24 2019, 9:17 PM
Restricted Application added a project: Plasma. · View Herald TranscriptOct 24 2019, 9:17 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
ngraham requested review of this revision.Oct 24 2019, 9:17 PM
apol accepted this revision.Oct 24 2019, 10:23 PM

Let's give it a try, merge to master, this could have side-effects.

discover/qml/DiscoverDrawer.qml
85

This shouldn't be necessary, is it?

This revision is now accepted and ready to land.Oct 24 2019, 10:23 PM
ngraham updated this revision to Diff 68726.Oct 24 2019, 11:11 PM

Remove unnecessary focus: true

ngraham marked an inline comment as done.Oct 24 2019, 11:11 PM

@apol does this make sense conceptually though? I don't want to commit a big ugly hack.

apol added a comment.Oct 25 2019, 12:16 AM

Sure, it makes sense. It's not really changing much, only removing the loader. The loader did make things slightly more complex so I can imagine this working better.

The whole Component.onCompleted: { bit is what gives it the focus.

Having the focus there (which is where it used to be in some older version) makes it a bit harder to navigate the central view with the arrows, so it's a matter of really really wanting to search first, which I think makes a lot of sense.

As is master right now, ctrl+f does give the focus to the search bar.

ngraham added a comment.Oct 25 2019, 12:29 AM
In D24935#553754, @apol wrote:

Having the focus there (which is where it used to be in some older version) makes it a bit harder to navigate the central view with the arrows

Hopefully that will eventually be fixed with Marco's work on https://bugs.kde.org/show_bug.cgi?id=407524

Closed by commit R134:5d1b3718efb8: Fix search field focus, both initial and on-demand (authored by ngraham). · Explain WhyOct 25 2019, 12:30 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mdiscover/qml/DiscoverDrawer.qml (82 lines)

Diff 68719

View Options

discover/qml/DiscoverDrawer.qml

Loading...