Add deselect action
Needs RevisionPublic

Authored by faridb on May 5 2019, 8:22 PM.

Details

Reviewers
elvisangelaccio
Group Reviewers
Dolphin
VDG
Summary

Add the ability to deselect selected elements.
This uses KStandardAction::deselect which is by default assigned to CTRL++A.

FEATURE: 184717

Diff Detail

Repository
R318 Dolphin
Branch
feature/deselect
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 11577
Build 11595: arc lint + arc unit
faridb created this revision.May 5 2019, 8:22 PM
Restricted Application added a project: Dolphin. · View Herald TranscriptMay 5 2019, 8:22 PM
Restricted Application added a subscriber: kfm-devel. · View Herald Transcript
faridb requested review of this revision.May 5 2019, 8:22 PM
faridb edited the summary of this revision. (Show Details)May 5 2019, 8:24 PM
ngraham accepted this revision as: VDG.May 5 2019, 8:48 PM
ngraham added reviewers: Dolphin, VDG.
ngraham added subscribers: elvisangelaccio, ngraham.

Works fine and LGTM, including the keyboard shortcut change.

@elvisangelaccio?

I am not familiar with this but shouldn't the "deselect" option be directly above the "invert selection" option or does it have no effect on the order ?

It already is above that action in the edit menu.

elvisangelaccio requested changes to this revision.May 12 2019, 10:56 AM

Sorry but this patch could break the workflow of those who use invert_selection. When we want to change a shortcut that's been there since forever, we need very good reasons and I'm afraid the deselect action is not enough:

  1. You can already deselect everything by pressing an empty space in the view, if you are a mouse user.
  2. You can already deselect everything by pressing the Esc key, if you are a keyboard user.
  3. Wish #184717 has been open for 10 years and has only 1 vote.
This revision now requires changes to proceed.May 12 2019, 10:56 AM
  1. You can already deselect everything by pressing an empty space in the view, if you are a mouse user.

This is not always possible. For example, if you're using the details view mode and there is not empty space to click (when the scrollbar is active).

  1. You can already deselect everything by pressing the Esc key, if you are a keyboard user.

I actually didn't know this.

What about adding the deselect action to the menu and disabling its default keyboard shortcut which conflicts with the shortcut for invert selection?
The deselect action will be available in the menu without being assigned a keyboard shortcut which will be useful for mouse users. Keyboard users are already able to use Esc.

  1. You can already deselect everything by pressing an empty space in the view, if you are a mouse user.

This is not always possible. For example, if you're using the details view mode and there is not empty space to click (when the scrollbar is active).

Can you show a screenshot of a detail view affected by such problem?

  1. You can already deselect everything by pressing the Esc key, if you are a keyboard user.

I actually didn't know this.

What about adding the deselect action to the menu and disabling its default keyboard shortcut which conflicts with the shortcut for invert selection?
The deselect action will be available in the menu without being assigned a keyboard shortcut which will be useful for mouse users. Keyboard users are already able to use Esc.

Hmm, I don't know, normal actions should always have a shortcut.

Can you show a screenshot of a detail view affected by such problem?

I just realised that you have to click exactly on the file/folder label to get it selected and clicking anywhere else deselects everything. I thought clicking on the row would select the file/folder but it doesn't. My bad.