[RenameDialog] Add an arrow indicating direction from src to dest
ClosedPublic

Authored by ahmadsamir on Apr 28 2020, 1:17 PM.

Details

Summary

This adds a visual indication to show the direction of the copy/move..etc
operation pointing from source to destination.

See the screenshots.


BUG: 268600

Diff Detail

Repository
R241 KIO
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
ahmadsamir created this revision.Apr 28 2020, 1:17 PM
Restricted Application added a project: Frameworks. · View Herald TranscriptApr 28 2020, 1:17 PM
ahmadsamir requested review of this revision.Apr 28 2020, 1:17 PM
ahmadsamir edited the summary of this revision. (Show Details)Apr 28 2020, 1:19 PM

I tried with Arabic, and the rename dialog had some Arabic text, but the layout was still LTR (can it switch to RTL?).

src/widgets/renamedialog.cpp
300

If you figure out how to make it RTL aware, you could toggle between the go-next-symbolic and go-next-rtl-symbolic icons

I meant it worked with Arabic, the direction was the same, just the text is translated, so the renamedialog and dolphin aren't RTL AFAICS.

ngraham accepted this revision.Apr 28 2020, 4:11 PM
This revision is now accepted and ready to land.Apr 28 2020, 4:11 PM
meven requested changes to this revision.Apr 29 2020, 7:14 AM
meven added inline comments.
src/widgets/renamedialog.cpp
142

You don't need a member variable for this.

This revision now requires changes to proceed.Apr 29 2020, 7:14 AM
ahmadsamir updated this revision to Diff 81492.Apr 29 2020, 8:44 AM

Don't use a member var for an object that's only used in one function

ahmadsamir marked an inline comment as done.Apr 29 2020, 8:44 AM
ahmadsamir added inline comments.
src/widgets/renamedialog.cpp
142

Indeed, it's only used in one place (too much copy/paste...).

meven accepted this revision.Apr 30 2020, 1:24 PM

LGTM

This revision is now accepted and ready to land.Apr 30 2020, 1:24 PM
pino added a subscriber: pino.Apr 30 2020, 1:32 PM
pino added inline comments.
src/widgets/renamedialog.cpp
299

this is not right-to-left aware; please use the layout direction of the widget to use "go-next" or "go-previous"

ngraham added inline comments.Apr 30 2020, 6:13 PM
src/widgets/renamedialog.cpp
299

(go-next-rtl-symbolic, not go-previous; that would be semantically inaccurate.)

ahmadsamir marked an inline comment as done.Apr 30 2020, 6:28 PM
ahmadsamir added inline comments.
src/widgets/renamedialog.cpp
299

@pino, right; dolphin isn't rtl-aware (or if it is, I couldn't find out how to switch it to rtl). But I agree the code here should account for rtl anyway.

@ngraham: I think we should stick to the icon naming spec[1], so that it works with themes other than breeze/oxygen; so it has to be go-previous.

[1] https://specifications.freedesktop.org/icon-naming-spec/icon-naming-spec-latest.html

hpereiradacosta added inline comments.
src/widgets/renamedialog.cpp
299

dolphin --reverse
should run it in right-to-left mode
(or whatever the opposite of your current locale)

pino added inline comments.Apr 30 2020, 6:33 PM
src/widgets/renamedialog.cpp
299

(or if it is, I couldn't find out how to switch it to rtl).

$ dolphin --reverse

it is provided directly by QApplication, it works also in Qt-only applications

src/widgets/renamedialog.cpp
299

you can use qApp->isRightToLeft() to check the rtl direction

safaalfulaij added inline comments.
src/widgets/renamedialog.cpp
299

The interface of Dolphin is RTL-aware, just the panels and the file view are not (for many usability reasons)
Make sure to have Qt translations installed if the layout isn't switching automatically with the language change.

OK, thank you :)

I'll adjust the diff accordingly.

ahmadsamir updated this revision to Diff 81627.Apr 30 2020, 7:37 PM

Take RTL layout into consideration when setting the arrow icon by using
qApp->isRightToLeft()

ahmadsamir updated this revision to Diff 81628.Apr 30 2020, 7:38 PM
ahmadsamir edited the summary of this revision. (Show Details)
ahmadsamir added a reviewer: Dolphin.

Verbatim

ahmadsamir edited the summary of this revision. (Show Details)Apr 30 2020, 7:39 PM
meven accepted this revision.May 1 2020, 5:03 PM
This revision was automatically updated to reflect the committed changes.