Details

Reviewers

ngraham
nicolasfella

Group Reviewers

Frameworks

Maniphest Tasks

T8552: Polish Open/Save dialogs

Commits

R241:10b6d83bbc4c: [KDirOperator] Use more human-readable sort order descriptions

Summary

Inspired by D22006

Test Plan

LANG=EN ctest

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.

meven created this revision.Jul 26 2019, 2:15 PM

Restricted Application added a project: Frameworks. · View Herald TranscriptJul 26 2019, 2:15 PM

Restricted Application added a subscriber: kde-frameworks-devel. · View Herald Transcript

meven requested review of this revision.Jul 26 2019, 2:15 PM

Harbormaster completed remote builds in B14447: Diff 62609.Jul 26 2019, 2:15 PM

meven edited the test plan for this revision. (Show Details)Jul 26 2019, 2:19 PM

Code looks sensible, haven't tried it yet

src/filewidgets/kdiroperator.cpp
1979	Remove commented code

Remove dead commented code

Harbormaster completed remote builds in B14449: Diff 62611.Jul 26 2019, 2:40 PM

Small formatting correction

Harbormaster completed remote builds in B14450: Diff 62612.Jul 26 2019, 2:41 PM

meven added inline comments.Jul 26 2019, 2:41 PM

src/filewidgets/kdiroperator.cpp
1979	Thanks ;)

meven marked an inline comment as done.Jul 26 2019, 2:41 PM

I tried this out but I'm not actually seeing the menu items in the open/save dialogs change at all. :/

In D22763#502738, @ngraham wrote:

I tried this out but I'm not actually seeing the menu items in the open/save dialogs change at all. :/

Have you tried with gwenview ?
For some reason, it does not work in kate but works in gwenview after installing kio, not event recompiling gwenview.

I have the same issue, I use the test app kfilewidgettest_saving_gui in kio tests dir for my tests usually.

I guess the old kio is kept in memory somewhere in kdeinit somewhere since kate uses it but not gwenview.
I have tried relaunching kdeinit5 but this does not seem to work.

Yes, it does work in Gwenview. How odd.

Anyway, not an issue with your patch, for sure. Patch LGTM!

This revision is now accepted and ready to land.Jul 26 2019, 4:36 PM

ngraham added a task: T8552: Polish Open/Save dialogs.Jul 26 2019, 4:36 PM

Remove unecessary call

Harbormaster completed remote builds in B14455: Diff 62628.Jul 27 2019, 7:12 AM

ngraham added inline comments.Jul 27 2019, 2:44 PM

src/filewidgets/kdiroperator.cpp
1992	Why change this? kdiroperator should definitely be ported to use the new connect syntax, but it should be done all at once in its own patch, not one at a time.

meven marked an inline comment as done.Jul 27 2019, 5:06 PM

meven added inline comments.

src/filewidgets/kdiroperator.cpp
1992	_k_slotSortReversed parameter means whether or not the sorting is reversed. Before this change, the descending action was a check box meaning its value should be the parameter to _k_slotSortReversed. But now the descending action is a radio button meaning when triggered it should always pass false to _k_slotSortReversed. A closure was the simple course of action to implement this, and could be reused as is for the ascending action as well. Onyl the new syntax allows closures to be used as slots.

meven marked 2 inline comments as done.Jul 27 2019, 5:07 PM

Shouldn't it be "Newest First" instead of "Newest first", to be consistent with other menu items (i.e. "Folders First", etc.)?

In D22763#503074, @churaev wrote:

Shouldn't it be "Newest First" instead of "Newest first", to be consistent with other menu items (i.e. "Folders First", etc.)?

Now that you mention it, I think so. I took the code from D22006

Capitalize as by https://hig.kde.org/style/writing/capitalization.html

Harbormaster completed remote builds in B14519: Diff 62711.Jul 29 2019, 6:56 AM

I will merge this beginning of next week, if no one objects.

Closed by commit R241:10b6d83bbc4c: [KDirOperator] Use more human-readable sort order descriptions (authored by meven). · Explain WhyAug 5 2019, 1:59 PM

This revision was automatically updated to reflect the committed changes.

Diff	ID	Base	Description	Created	Lint	Unit
Base			Base
Diff 1	62609	22179df		Jul 26 2019, 2:15 PM	★	★
Diff 2	62611	22179df	Remove dead commented code	Jul 26 2019, 2:40 PM	★	★
Diff 3	62612	22179df	Small formatting correction	Jul 26 2019, 2:41 PM	★	★
Diff 4	62628	22179df	Remove unecessary call	Jul 27 2019, 7:12 AM	★	★
Diff 5	62711	22179df	Capitalize as by https://hig.kde.org/style/writing/capitalization.html	Jul 29 2019, 6:56 AM	★	★
Diff 6	63122	7c840b9	R241:10b6d83bbc4cd0e5fecc6136a68fba3685a4d466	Aug 5 2019, 1:58 PM	★	★

[KDirOperator] Use more human-readable sort order descriptions
ClosedPublic
Actions

Details

Diff Detail

Revision Contents
Changeset List

Diff 63122

src/filewidgets/kdiroperator.cpp

[KDirOperator] Use more human-readable sort order descriptionsClosedPublicActions

Details

Diff Detail

Revision ContentsChangeset List

Diff 63122

src/filewidgets/kdiroperator.cpp

[KDirOperator] Use more human-readable sort order descriptions
ClosedPublic
Actions

Revision Contents
Changeset List