Improve activity list highlighting/keyboard item selection

[claucambra]

This PR improves our keyboard navigation of activity list items in the activity list (especially in terms of highlights).

This partially addresses #4220 (navigation within activity list items, such as to highlight action buttons, is still a bit funky)

https://user-images.githubusercontent.com/70155116/181247416-a3a07e65-24e6-4ff3-bf0b-9f3929f10dbf.mov

Signed-off-by: Claudio Cambra [email protected]

[codecov[bot]]

Codecov Report

Merging #4781 (6c52480) into master (2e2ed97) will decrease coverage by 0.01%. The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #4781      +/-   ##
==========================================
- Coverage   57.23%   57.22%   -0.02%     
==========================================
  Files         138      138              
  Lines       17144    17144              
==========================================
- Hits         9812     9810       -2     
- Misses       7332     7334       +2     

Impacted Files	Coverage Δ
src/libsync/discovery.cpp	84.56% <0.00%> (-0.30%)	:arrow_down:
src/libsync/propagatedownload.cpp	65.03% <0.00%> (-0.15%)	:arrow_down:
src/libsync/vfs/cfapi/cfapiwrapper.cpp	74.71% <0.00%> (+0.45%)	:arrow_up:

[claucambra]

@nimishavijay @jancborchardt what do you think about keyboard highlights? Should they be different from mouse-hovering highlights? At the moment I have made them the same (on keyboard navigation un clickable list items have an outline instead)

[jancborchardt]

@claucambra same style is good – I'd also say that the outline is not necessary but rather confusing. Can just be the same style. Ideally every entry has an action (in the case of your todos opening the Tasks app for example) but that's of course a server/app issue.

Can the action buttons be focused too?

[claucambra]

@claucambra same style is good – I'd also say that the outline is not necessary but rather confusing. Can just be the same style. Ideally every entry has an action (in the case of your todos opening the Tasks app for example) but that's of course a server/app issue.

Sorry, to clarify, the outline is only for list items that are un-clickable -- the intention behind the outline was distinguishing between an item that is "enabled" (i.e. clicking has some effect) vs. one that isn't. On hover with the mouse, these items aren't highlighted in any way, but with keyboard navigation you'd still need some indication that the outlined item is the one currently selected. Whenever we get, say, todos to be clickable and have them open the tasks app, the highlight will not be the outline but the full background change.

Having said all of this, I can change the highlight to be just like the mouse hover even on disabled items if that is preferable :)

Can the action buttons be focused too?

Under the hood the code relating to the buttons is a mess and making them focus-able in a coherent/predictable/functional way will require some serious refactoring. I've started with #4784 but it will likely take several more PRs of cleaning up things component by component. So I decided to keep this PR to just fixing keyboard navigation in the list view itself, but intra-listitem navigation is being worked on :)

[claucambra]

@claucambra same style is good – I'd also say that the outline is not necessary but rather confusing. Can just be the same style. Ideally every entry has an action (in the case of your todos opening the Tasks app for example) but that's of course a server/app issue.

Sorry, to clarify, the outline is only for list items that are un-clickable -- the intention behind the outline was distinguishing between an item that is "enabled" (i.e. clicking has some effect) vs. one that isn't. On hover with the mouse, these items aren't highlighted in any way, but with keyboard navigation you'd still need some indication that the outlined item is the one currently selected. Whenever we get, say, todos to be clickable and have them open the tasks app, the highlight will not be the outline but the full background change.

@jancborchardt @nimishavijay thoughts on the focus effect on pressable vs unpressable buttons? :)

[mgallien]

@claucambra same style is good – I'd also say that the outline is not necessary but rather confusing. Can just be the same style. Ideally every entry has an action (in the case of your todos opening the Tasks app for example) but that's of course a server/app issue.

Sorry, to clarify, the outline is only for list items that are un-clickable -- the intention behind the outline was distinguishing between an item that is "enabled" (i.e. clicking has some effect) vs. one that isn't. On hover with the mouse, these items aren't highlighted in any way, but with keyboard navigation you'd still need some indication that the outlined item is the one currently selected. Whenever we get, say, todos to be clickable and have them open the tasks app, the highlight will not be the outline but the full background change.

Having said all of this, I can change the highlight to be just like the mouse hover even on disabled items if that is preferable :)

I fear that the current implementation is pushing an implementation detail into the user if I want to scroll through the list, do I really care if something is user interactive or not ? when interacting with a mouse, I can try to click to see if this is interactive or not and interacting via a screen reader can be done in the same way (and visually impaired people will not really see the visual distinction anyway)

[jancborchardt]

Yup, agree with @mgallien – and as said, would be less confusing if the focus highlight is the same as for hover. :)

[claucambra]

Okay @jancborchardt the highlight is now the same on all activity items

https://user-images.githubusercontent.com/70155116/183869627-3e0bdc60-7bcc-4cd7-8681-c33dd8805a2f.mov

[sonarqubecloud[bot]]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
No Duplication information

[nextcloud-desktop-bot]

AppImage file: nextcloud-PR-4781-b820df90b9a400f7791e1c62cad1d657293e3de5-x86_64.AppImage

To test this change/fix you can simply download above AppImage file and test it.

Please make sure to quit your existing Nextcloud app and backup your data.