When renaming a file, it's new name causes it to scroll out of view Dolphin will not scroll to the location of the new file. This patch aims to address that. This affects all view modes.
BUG: 354330
Details
Details
- Reviewers
elvisangelaccio ngraham emmanuelp - Group Reviewers
Dolphin - Commits
- R318:478f404b8abf: Keep renamed file(s) in view
- Have many files in a directory (or several files, just zoom in a lot)
- Rename a file so that it will move out of view
Diff Detail
Diff Detail
- Repository
- R318 Dolphin
- Lint
Automatic diff as part of commit; lint not applicable. - Unit
Automatic diff as part of commit; unit tests not applicable.
| src/views/dolphinview.cpp | ||
|---|---|---|
| 1766 | This is called only when the user clicks the Rename button and items were actually renamed. | |
Comment Actions
Please remove BUG: .... from the summary line, and replace the line
Relevant bug report: https://bugs.kde.org/show_bug.cgi?id=354330
with
BUG: 354330
| src/views/dolphinview.cpp | ||
|---|---|---|
| 1762 | The old code did not have this assignment. Is it really necessary? | |
| src/views/dolphinview.h | ||
| 315 | I'd just call it slotRenamingFinished(). Also, no need to have it public, make it a private slot. | |
| src/views/renamedialog.cpp | ||
| 42 | Not really necessary, default QList ctor is automatically called. | |
| 246 | Unrelated whitespace change. | |
| src/views/dolphinview.h | ||
|---|---|---|
| 771 | Maybe forceUrlsSelection() would be more clear | |
| src/views/dolphinview.cpp | ||
|---|---|---|
| 1762 | As far as I remember until I added that statement when mass renaming the new selection after the rename would include some (seemingly random) items in the directory but I cannot reproduce it reliably. | |
Comment Actions
-1
FWIW, I don't approve of this patch from a usability perspective. It's not at all clear that changing the view is what the user expects in this case. Especially if you're renaming multiple files in such a manner that they will be re-sorted elsewhere, the view will change after each rename operation and you will need to manually scroll back over and over again, making the multiple-file-rename operation an infuriating experience. macOS finder behaved this way for many years and Apple got a lot of complaints about it, eventually reverting the behavior to match what Dolphin does today.
Comment Actions
You could argue the same with pasting. If you paste or drop a file, the Dolphin view will scroll to it.
This patch just fixes a regression/inconsistency, as the feature was already there.
Renaming a single file is probably a more common use case, so I think is safe if Dolphin privileges it over multiple successive renames. The latter use case could be fixed in other ways (for example, using the new stash:/ ioslave).
| src/views/dolphinview.cpp | ||
|---|---|---|
| 1754 | No, it will be called only when there are successful rename operations finished. | |
Comment Actions
Looks good to me now.
@emmanuelp can you have another look? :)
| src/views/renamedialog.cpp | ||
|---|---|---|
| 254 | Unrelated whitespace change. | |
Comment Actions
@elvisangelaccio @emmanuelp since @emateli doesn't have commit access should I commit?
To which branch?
Comment Actions
I think it should go to the stable branch. I'll take care of it, I need to push another patch anyway :)
Comment Actions
@emateli You may be interested in fixing https://bugs.kde.org/show_bug.cgi?id=354330#c8 ?
Comment Actions
@elvisangelaccio: So I was thinking of using KIO::FileUndoManager::undoJobFinished but it unfortunately provides no details as to what job was undone.
A possible option would be to keep the last job recorded so we have the name that the undo-ed item will have after reverting, but this is just hacky and doesn't support more than 1 action. I think best would be if job information was passed from KIO::FileUndoManager and then Dolphin could leverage it. I also tried to use selectionManager but to no avail.
Comment Actions
Yeah, looks like we need a new signal in KIO::FileUndoManager that tells us which job was undone...