[KIO/RenameDialog] Add new apply behaviour
Needs RevisionPublic

Authored by chinmoyr on Feb 3 2019, 3:53 PM.

Details

Reviewers
dfaure
ngraham
Group Reviewers
VDG
Summary

This patch removes "Apply to All" checkbox and adds a combobox with following 4 apply options:

  1. Apply to Current : Same as "Apply to All" unchecked.
  2. Apply to All
  3. Apply to Newer Dest : Applies changes (currently only overwrite) to conflicts with a newer version of dest file/dir. And skip the rest.
  4. Apply to Older Dest : Same as above but with older dest.

In this version of patch when selecting the last two options only "Overwrite"/"Write Into" button is enabled.

As a result this patch also modifies CopyJob to implement the new overwrite behaviour.

BUG: 94670

Diff Detail

Repository
R241 KIO
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 7815
Build 7833: arc lint + arc unit
chinmoyr created this revision.Feb 3 2019, 3:53 PM
Restricted Application added a project: Frameworks. · View Herald TranscriptFeb 3 2019, 3:53 PM
Restricted Application added a subscriber: kde-frameworks-devel. · View Herald Transcript
chinmoyr requested review of this revision.Feb 3 2019, 3:53 PM
chinmoyr updated this revision to Diff 50778.Feb 3 2019, 3:59 PM
chinmoyr edited the summary of this revision. (Show Details)

Add bug id.
Remove old code.

broulik added a subscriber: broulik.Feb 3 2019, 4:03 PM

I find the wording and usability a bit confusing, "Older Dest", "Newer Dest", etc. I think that dialog needs to be overhauled entirely for this approach to work, cf. to how windows uses command links for that (they're also quite hard to grasp at a glance imho), not sure, perhaps usability knows a good solution for this problem.

chinmoyr added a reviewer: VDG.Feb 3 2019, 4:16 PM
cfeck added a subscriber: cfeck.EditedFeb 3 2019, 4:45 PM

The "Apply to All" button is used for all choices, including "Overwrite", "Skip", and "Rename".

NVM, I should have looked at the pictures, not only at the "Removed checkbox" description...

dfaure requested changes to this revision.Mar 2 2019, 11:42 PM

Yeah that naming needs improvement. At the very least, using a short form like "Dest" doesn't seem like a good idea in a user-facing GUI.

This revision now requires changes to proceed.Mar 2 2019, 11:42 PM
rooty added a subscriber: rooty.EditedMar 3 2019, 12:05 AM

I propose that "Apply to" be taken out of the checkbox/combobox, hence Apply to: "combobox"

And I propose the labels:
"This File Only"
"Every File" (or Every File in Transit)
"Destination, if Older than Source"
"Destination, if More Recent than Source"

Otherwise, nice patch! +1

P.S. I know they're "wordy" but the options are actually very complex :D I think we can risk it. The problem is that if it says "Older Dest" it sounds like a destination that's older than some other destination and not the file being copied.

You could use "Destination, if Older than Source" and "Destination, if More Recent than Source"
OR use the source as a reference point, hence
"Destination if Source is More Recent" and "Destination if Source is Older", respectively.