[Kickoff] Make the visible search field unfocused by default
AbandonedPublic

Authored by rooty on Nov 18 2018, 8:48 PM.

Details

Reviewers
romangg
ngraham
davidedmundson
mart
Group Reviewers
Plasma
VDG
Summary

Changes: applets/kickoff/package/contents/ui/Header.qml

This patch:
(1) Keeps the search field visible by default but leaves it unfocused until triggered by the keyboard
(2) Enables the tab key to select/focus the search field just like any other key
(3) Enables the escape key to collapse Kickoff when pressed from within an empty focused search field that was activated by either a mouse click
or the tab key (a bug in the previous version of Kickoff evident due to an unfocused search area)
(4) Enables the escape key to return the user to the tabbed view in Kickoff when pressed from within a non-empty search field
(5) Changes the label 'Search...' to 'Type to search...' so that the user is made aware that they can, in fact, type to search
(6) Maintains the ability to hover over the user's name to show more information about the system
(7) Lowers the font size of the username to make it fit in better with the search field and user picture/avatar

Test Plan



Diff Detail

Repository
R119 Plasma Desktop
Branch
kickoff-visible-and-unfocused (branched from master)
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 5140
Build 5158: arc lint + arc unit
rooty created this revision.Nov 18 2018, 8:48 PM
Restricted Application added a project: Plasma. · View Herald TranscriptNov 18 2018, 8:48 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
rooty requested review of this revision.Nov 18 2018, 8:48 PM
rooty edited the test plan for this revision. (Show Details)Nov 18 2018, 8:54 PM
rooty edited the test plan for this revision. (Show Details)
rooty edited the test plan for this revision. (Show Details)Nov 18 2018, 8:57 PM
rooty edited the test plan for this revision. (Show Details)
rooty updated this revision to Diff 45750.Nov 18 2018, 9:05 PM
  • Remove unnecessary states, tidy things up
rooty updated this revision to Diff 45751.Nov 18 2018, 9:07 PM
  • Tidy up some more
rooty updated this revision to Diff 45753.Nov 18 2018, 9:13 PM
  • Fix broken states and return ability to hover
filipf added a subscriber: filipf.Nov 18 2018, 9:39 PM

I cannot comment on the technical side of things (noob), and am perhaps a little biased since Root and I are best friends who have been working (mostly him) on Kickoff this weekend as our first real contribution to KDE, however... this change can really do wonders for helping the search bar blend in throughout various themes and should be the final nail in the coffin of that debate. It helps a lot with transparent themes:


(here depicted: the Opal desktop theme)

It also conceptually makes more sense to have the search field unfocused because searching is not necessarily the main activity done within Kickoff, so why have it in focus all the time?

rooty updated this revision to Diff 45757.Nov 18 2018, 10:12 PM
  • Trim unnecessary code
rooty edited the test plan for this revision. (Show Details)Nov 18 2018, 11:12 PM
ngraham requested changes to this revision.Nov 19 2018, 3:09 AM
ngraham added subscribers: chempfling, gladhorn.

There are a lot of good changes in here, like those keyboard navigation improvements! And I appreciate all the work that clearly went into this. However, the problem with huge patches like this is that if we like some but not all of it, you end up needing to re-work a lot of the patch. That's generally why we prefer "atomic" changes with one change per patch/commit. It makes that kind of thing way saner.

And I'm afraid I don't think we can do #1. @chempfling and @gladhorn have been working hard to push on accessibility, and one thing I've learned in the past week weeks is how important focus is. Making sure that the active element both has and looks like it has focus is critical to making sure the UI is accessible for screen readers. As such, we need to keep it visually focused by default when it opens.

I would be open to making the search field behave like it does in Kicker: visually focused by default, but then focus shifts to list items when they're active (but you can still type-to-search anywhere).

One more note: while compatibility with 3rd-party themes is a laudable goal, it can never be a primary one, and it can't dictate design considerations. 3rd-party themes are a moving target and chasing them is futile, so "this change makes it look better with 3rd-party themes" can never be a selling point. It has to be better for Breeze first and foremost. If the change also improves the look for 3rd-party themes, all the better, but that can't be the primary consideration.

So I'd like to see the font size changes go into one patch, the keyboard navigation improvements go into a second, and the search field placeholder text change in a third one. I think those are fairly non-controversial.

This revision now requires changes to proceed.Nov 19 2018, 3:09 AM

And I'm afraid I don't think we can do #1. @chempfling and @gladhorn have been working hard to push on accessibility, and one thing I've learned in the past week weeks is how important focus is. Making sure that the active element both has and looks like it has focus is critical to making sure the UI is accessible for screen readers. As such, we need to keep it visually focused by default when it opens.

I would be open to making the search field behave like it does in Kicker: visually focused by default, but then focus shifts to list items when they're active (but you can still type-to-search anywhere).

One more note: while compatibility with 3rd-party themes is a laudable goal, it can never be a primary one, and it can't dictate design considerations. 3rd-party themes are a moving target and chasing them is futile, so "this change makes it look better with 3rd-party themes" can never be a selling point. It has to be better for Breeze first and foremost. If the change also improves the look for 3rd-party themes, all the better, but that can't be the primary consideration.

Hmm but it doesn't even need to be primarily argumented as a visual improvement. Can't we approach the focused/unfocused matters contextually? If I'm using Kickoff mostly to click on my favorites and use the leave actions, why should the search field be glowing and be at 100% opacity all the time?

For comparison's sake and looking at our major competitors, there is no such thing in Windows - there's even no search field in the menu and the Cortana tab on the panel which can act as a replacement isn't focused until you interact with it. In GNOME, which is even more incentivized to keep the search field focused, that doesn't happen either and the field looks subtle. From what I'm seeing, in macOS the Launchpad's search bar is even subtler than in GNOME and sans a blinking cursor that can be activated, there isn't even such a thing as a focused state. I also see that Spotlight search, which is actually something like KRunner (so something made solely for searching) is pretty subtle looking and also doesn't have a focus state. And I notice they've got search fields everywhere and none of them are focused unless interacted with. The Deepin search bar likewise blends in seamlessly into the menu and doesn't seem to have a focused state either. Budgie only has an underline of sorts as its one and only state. The Cinnamon menu has only one state and it's subtle looking. In short, seems like a lot of desktop environments make do just fine without making the search bar stick out.

Taking that into account, plus the fact that we didn't even have a search field for 4 years — and that this hasn't been complained about that unanimously and vocally — is it really that imperative to keep it focused?

Hmm but it doesn't even need to be primarily argumented as a visual improvement. Can't we approach the focused/unfocused matters contextually? If I'm using Kickoff mostly to click on my favorites and use the leave actions, why should the search field be glowing and be at 100% opacity all the time?

Right, I agree. In fact, that's how Kicker does it: the search field is focused by default, but if you mouse over a list item, then focus visually moves to the list. I'm supportive of doing that sort of thing here. Then the Kicker and Kickoff search fields would even be consistent with one another, which would be a nice benefit.

rooty added a comment.EditedNov 19 2018, 7:18 AM

And I'm afraid I don't think we can do #1. @chempfling and @gladhorn have been working hard to push on accessibility, and one thing I've learned in the past week weeks is how important focus is. Making sure that the active element both has and looks like it has focus is critical to making sure the UI is accessible for screen readers. As such, we need to keep it visually focused by default when it opens.

While this is a very valid point (and I don't dislike the already focused search field), the verb type in the imperative mood is the most efficient way to inform the user that they can search. The UI seeming accessible if focused is more of an intuitive leap (not a bad one though). Spotlight, for instance, does this by means of a magnifying glass and shaded lettering, and it moves onto the focused search results by default, but it doesn't focus the search field - there seems to be little benefit in keeping the actual search field focused unless it's being used (it's too obvious). That's why I was tinkering with the idea of removing the focus altogether (but I do like the blue so...).

This is also similar to how Clipboard behaves.

One more note: while compatibility with 3rd-party themes is a laudable goal, it can never be a primary one, and it can't dictate design considerations. 3rd-party themes are a moving target and chasing them is futile, so "this change makes it look better with 3rd-party themes" can never be a selling point. It has to be better for Breeze first and foremost. If the change also improves the look for 3rd-party themes, all the better, but that can't be the primary consideration.

Futile? While not primary, it should still be a major concern because KDE is touted as endlessly customizable.

So I'd like to see the font size changes go into one patch, the keyboard navigation improvements go into a second, and the search field placeholder text change in a third one. I think those are fairly non-controversial.

While in principle I do agree, that's not entirely doable here without making a decision about whether the search field is unfocused or not. Splitting up the patch means that the keyboard improvements go down the drain (Esc is used to escape no matter what (this works already) and there's no need to use Tab to select the search field (it's already in focus)) because they aren't necessary. The font changes can be adapted into a separate patch, though.

rooty added a comment.EditedNov 19 2018, 7:41 AM

Right, I agree. In fact, that's how Kicker does it: the search field is focused by default, but if you mouse over a list item, then focus visually moves to the list. I'm supportive of doing that sort of thing here. Then the Kicker and Kickoff search fields would even be consistent with one another, which would be a nice benefit.

Not bad. I'd have to rework this Header.qml to make it more like Kicker though. But that's only if everyone's really sure they don't want the unfocused search field.

By the way, can I place all the font (and hover) changes into a single patch?

P.S. I'm on board with the unfocused search field thing but you're playing on my heartstrings with Kicker haha... (I like it a lot because it's the perfect complement to the global menu.)

There are a lot of good changes in here, like those keyboard navigation improvements! And I appreciate all the work that clearly went into this. However, the problem with huge patches like this is that if we like some but not all of it, you end up needing to re-work a lot of the patch. That's generally why we prefer "atomic" changes with one change per patch/commit. It makes that kind of thing way saner.

And I'm afraid I don't think we can do #1. @chempfling and @gladhorn have been working hard to push on accessibility, and one thing I've learned in the past week weeks is how important focus is. Making sure that the active element both has and looks like it has focus is critical to making sure the UI is accessible for screen readers. As such, we need to keep it visually focused by default when it opens.

Actually this sounds like it might help our cause. Especially when the search bar can be tabbed into, that's really nice. The problem is that we would like to focus to be on the selected item, such as the one that gets launched when pressing enter. The search should not have the focus unless the user type. I didn't look at the code change at all, but from the description it may just be a step forward.

rooty added a comment.Nov 19 2018, 9:27 AM

Actually this sounds like it might help our cause. Especially when the search bar can be tabbed into, that's really nice. The problem is that we would like to focus to be on the selected item, such as the one that gets launched when pressing enter. The search should not have the focus unless the user type. I didn't look at the code change at all, but from the description it may just be a step forward.

This did occur to me too, and it's a great idea, I just haven't figured out how to implement it yet haha

rooty added a comment.EditedNov 19 2018, 9:34 AM

@gladhorn More food for thought by the way: in this patch (and the master) the search field is focused while searching and while navigating the search results - it still accepts input even though you've technically navigated away from it. Should this functionality be kept (regardless of if the search field remains visibly focused or not), or should we prevent the user from typing into the search field while the focus is on the search results?

I think it's a good idea to let the user type to search from wherever they may be in Kickoff, but to draw the focus away from the search field (like Kicker).

@gladhorn More food for thought by the way: in this patch (and the master) the search field is focused while searching and while navigating the search results - it still accepts input even though you've technically navigated away from it. Should this functionality be kept (regardless of if the search field remains visibly focused or not), or should we prevent the user from typing into the search field while the focus is on the search results?

I think it's a good idea to let the user type to search from wherever they may be in Kickoff, but to draw the focus away from the search field (like Kicker).

Yes, I want to be able to type, no matter where, but the search result should get the focus in my opinion. We don't want to lose that functionality, it's about knowing what the currently most relevant item is, just like a sighted user would see the top list item. Then if the arrow keys for example work to go to the next search result, just like you'd glance down, then that's perfect.

rooty added a comment.EditedNov 19 2018, 11:08 AM

Okay so from what I can tell:

(1) Kicker takes the focus off the search field when you go through the results by either mouse or keyboard. This makes it easy to go back to the search field by typing or using the arrow keys (to add more words etc.)
(2) Krunner becomes unfocused if you use the arrow keys, but not if you hover over the search results with your mouse, and it's still possible in either case to append your search same as in Kickoff.
(3) Kickoff (the old, the master and this patch) seems to leave the search field focused at all times while you go through the results, and it's possible to add more characters to the search like in (1) and (2).
(4) Clipboard behaves pretty much like this patch except pressing the Esc key doesn't make it unfocused again.

Any thoughts on how to reconcile all these differences? It might take some doing 😅

Okay so from what I can tell:

(1) Kicker takes the focus off the search field when you go through the results by either mouse or keyboard. This makes it easy to go back to the search field by typing or using the arrow keys (to add more words etc.)
(2) Krunner becomes unfocused if you use the arrow keys, but not if you hover over the search results with your mouse, and it's still possible in either case to append your search same as in Kickoff.
(3) Kickoff (the old, the master and this patch) seems to leave the search field focused at all times while you go through the results, and it's possible to add more characters to the search like in (1) and (2).
(4) Clipboard behaves pretty much like this patch except pressing the Esc key doesn't make it unfocused again.

Any thoughts on how to reconcile all these differences? It might take some doing 😅

Howdy,

just for clearification :). With focus and unfocuse we talk about "focus: true", not only an highlighting effect?

Okay so from what I can tell:

(1) Kicker takes the focus off the search field when you go through the results by either mouse or keyboard. This makes it easy to go back to the search field by typing or using the arrow keys (to add more words etc.)
(2) Krunner becomes unfocused if you use the arrow keys, but not if you hover over the search results with your mouse, and it's still possible in either case to append your search same as in Kickoff.
(3) Kickoff (the old, the master and this patch) seems to leave the search field focused at all times while you go through the results, and it's possible to add more characters to the search like in (1) and (2).
(4) Clipboard behaves pretty much like this patch except pressing the Esc key doesn't make it unfocused again.

Any thoughts on how to reconcile all these differences? It might take some doing 😅

Howdy,

just for clearification :). With focus and unfocuse we talk about "focus: true", not only an highlighting effect?

Yup - not just a cosmetic effect

I like all the changes, but they should go into separate patches:

(1) Keeps the search field visible by default but leaves it unfocused until triggered by the keyboard

Patch 1

(2) Enables the tab key to select/focus the search field just like any other key

Patch 2

(3) Enables the escape key to collapse Kickoff when pressed from within an empty focused search field that was activated by either a mouse click
or the tab key (a bug in the previous version of Kickoff evident due to an unfocused search area)
(4) Enables the escape key to return the user to the tabbed view in Kickoff when pressed from within a non-empty search field

Patch 3

(5) Changes the label 'Search...' to 'Type to search...' so that the user is made aware that they can, in fact, type to search
(6) Maintains the ability to hover over the user's name to show more information about the system
(7) Lowers the font size of the username to make it fit in better with the search field and user picture/avatar

Patch 4

You could use this diff here for patch 1 and split the other changes into dependent diffs.

(1) Kicker takes the focus off the search field when you go through the results by either mouse or keyboard. This makes it easy to go back to the search field by typing or using the arrow keys (to add more words etc.)

Why does it make it easy?

(2) Krunner becomes unfocused if you use the arrow keys, but not if you hover over the search results with your mouse, and it's still possible in either case to append your search same as in Kickoff.
(3) Kickoff (the old, the master and this patch) seems to leave the search field focused at all times while you go through the results, and it's possible to add more characters to the search like in (1) and (2).
(4) Clipboard behaves pretty much like this patch except pressing the Esc key doesn't make it unfocused again.

Every applet does it differently. Great.

I would propose:

  • By default search field looks unactivated but has focus. Arrow up/down sets focus to the item selected, but typing to start a search is still possible.
  • While typing input field and current top item look activated, but focus shall not lie on the input field but on the item which would get started when Enter is pressed (this item could change while continuing to type).
  • When moving in the search results with arrow keys up/down the search field stays activated (but still the focus lies on the currently selected item).
  • One does not need to go back to the search field with arrow keys to add/remove characters (and also should not be able to do that like in Kicker).
rooty added a comment.Nov 19 2018, 2:30 PM

Why does it make it easy?

Sorry I have no idea what I was thinking when I wrote that. It doesn't. (I think my point was that it works regardless of the focus being drawn away from it?)

  • By default search field looks unactivated but has focus. Arrow up/down sets focus to the item selected, but typing to start a search is still possible.

Clipboard works like this but the coding is beyond my abilities. Krunner does this too except it opens up with the search field in focus. This Kickoff patched version can't do that, and I've yet to figure out how to make it be able to.

  • One does not need to go back to the search field with arrow keys to add/remove characters (and also should not be able to do that like in Kicker).

It's not necessary to do so in Kicker. You can just keep on typing.

rooty added a comment.Nov 19 2018, 2:31 PM

I like all the changes, but they should go into separate patches:

Patches that would patch this patch or patches that would each patch master separately?

Clipboard works like this but the coding is beyond my abilities. Krunner does this too except it opens up with the search field in focus. This Kickoff patched version can't do that, and I've yet to figure out how to make it be able to.

I believe in you! :) If you need quick help come join the Plasma IRC channel.

It's not necessary to do so in Kicker. You can just keep on typing.

I know it's not necessary. I meant that it should not be allowed. My argument is there is no purpose/advantage to that if you can just continue typing from every focused item. All it does is making it one more item to cycle through when moving up/down through the search results.

Patches that would patch this patch or patches that would each patch master separately?

You can do this as you see fit. I would recommend though in this case to make a patch series where patches depend on each other since otherwise it can become difficult to keep all the independent patches consistent with each other.

rooty added a comment.EditedNov 19 2018, 2:50 PM

You can do this as you see fit. I would recommend though in this case to make a patch series where patches depend on each other since otherwise it can become difficult to keep all the independent patches consistent with each other.

Excellent.

I believe in you! :) If you need quick help come join the Plasma IRC channel.

Thanks! :D I just have to figure out how IRC works. And I thought arc was a challenge 🤣

Cool, I'm looking forward to most of these changes. And you may yet convince me on the visually-unfocused-search-field-by-default thing. :)

Here's the documentation for creating a dependency chain of patches: https://community.kde.org/Infrastructure/Phabricator#Marking_patches_as_dependent_on_other_patches

rooty added a comment.Nov 19 2018, 3:27 PM

Thanks!

Ironically, I do like it focused too 🤣

rooty updated this revision to Diff 45820.Nov 19 2018, 4:09 PM
  • [Kickoff] Make search field unfocused but visible
mart requested changes to this revision.EditedNov 23 2018, 11:35 AM
mart added a subscriber: mart.

that weird focus policy probably messes with screen readers for accessibility

This revision now requires changes to proceed.Nov 23 2018, 11:35 AM
mart added a comment.Nov 23 2018, 11:37 AM

It also conceptually makes more sense to have the search field unfocused because searching is not necessarily the main activity done within Kickoff, so why have it in focus all the time?

is important to have the workflow open the menu + just start typing intact to put search at the center of the ui

D17020, D17022, and D17023 still include duplicated changes from this patch.

The idea was to make all of your Kickoff patches independent of each other, or at least to thin out the dependency tree so that all the patches depend on a single ancestor patch that only has uncontroversial refactoring changes. Right now the dependency on this one means that all of your other much-less-controversial changes are gated on getting acceptance for making the search field visually unfocused by default.

Am I making any sense?

rooty added a comment.Nov 29 2018, 9:25 AM

D17020, D17022, and D17023 still include duplicated changes from this patch.

The idea was to make all of your Kickoff patches independent of each other, or at least to thin out the dependency tree so that all the patches depend on a single ancestor patch that only has uncontroversial refactoring changes. Right now the dependency on this one means that all of your other much-less-controversial changes are gated on getting acceptance for making the search field visually unfocused by default.

Am I making any sense?

No. What you're saying makes a kind of sense until you actually think about it - while it's possible to change D17023 to make it more discrete (it's a font size and label change, basically three lines of code), even that makes little sense in the face of a focused search field that would be present without all the other patches. It's not possible to make D17022 or D17020 (re-enable hover) independent because they quite literally depend on an unfocused search field and are bug fixes for issues that this here diff introduces. So it makes very little if any sense to make them independent (the current Kickoff doesn't have these bugs).

Suffice it to say, you can't use any of the other patches (except the font size / label change) without this one.

P.S. If you want to reject the patch based on controversy, that's fine with me. I don't care much about the focus anyway tbh

Suffice it to say, you can't use any of the other patches (except the font size / label change) without this one.

Separating this doesn't make sense either because there's no need to change "Search..." to "Type to search..." if the field will remain focused as it is now.

rooty added a comment.Nov 29 2018, 9:41 AM

Suffice it to say, you can't use any of the other patches (except the font size / label change) without this one.

Separating this doesn't make sense either because there's no need to change "Search..." to "Type to search..." if the field will remain focused as it is now.

good point. so.... they're entirely dependent.

rooty abandoned this revision.Jan 2 2019, 11:31 PM

Seeing as this causes the latest plasmashell to lock up and I was never too enthusiastic about the change either, I'm going to close this, sorry guys.

Seeing as this causes the latest plasmashell to lock up

Can you expand on this? What locks up?

I had a bug report this week of a guy who had kickoff locking up on pressing tab