D12337: Give the file dialogs a "Sort by" menu button on the toolbar

ngraham edited the test plan for this revision. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D12337 To: ngraham, #frameworks, #dolphin, #vdg, rkflx Cc: kde-frameworks-devel, andreaska, markg, broulik, anemeth, michaelh, ngraham, bruns

D12337: Give the file dialogs a "Sort by" menu button on the toolbar

ngraham added a comment. Done. I've created the `file-dialog-improvements` branch and landed this there. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D12337 To: ngraham, #frameworks, #dolphin, #vdg, rkflx Cc: kde-frameworks-devel, andreaska, markg, broulik, anemeth,

D12337: Give the file dialogs a "Sort by" menu button on the toolbar

ngraham closed this revision. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D12337 To: ngraham, #frameworks, #dolphin, #vdg, rkflx Cc: kde-frameworks-devel, andreaska, markg, broulik, anemeth, michaelh, ngraham, bruns

D12337: Give the file dialogs a "Sort by" menu button on the toolbar

ngraham removed a dependency: D12333: Put the open/save dialog's toolbar above all other widgets, like Dolphin does. REPOSITORY R241 KIO BRANCH arcpatch-D12337 REVISION DETAIL https://phabricator.kde.org/D12337 To: ngraham, #frameworks, #dolphin, #vdg, rkflx Cc: kde-frameworks-devel,

D12337: Give the file dialogs a "Sort by" menu button on the toolbar

rkflx added a comment. Let's commit this on a separate branch, as getting micro-approvals from #Frameworks does not work that well. Then we can implement (in the form of reviewed Diffs as before) what we already discussed in a bigger setting,

D12337: Give the file dialogs a "Sort by" menu button on the toolbar

ngraham edited the summary of this revision. ngraham edited the test plan for this revision. REPOSITORY R241 KIO BRANCH arcpatch-D12337 REVISION DETAIL https://phabricator.kde.org/D12337 To: ngraham, #frameworks, #dolphin, #vdg, rkflx Cc: kde-frameworks-devel, andreaska, markg, broulik,

D12337: Give the file dialogs a "Sort by" menu button on the toolbar

ngraham marked an inline comment as done. ngraham added a comment. Any opinions from #frameworks folks? REPOSITORY R241 KIO BRANCH arcpatch-D12337 REVISION DETAIL https://phabricator.kde.org/D12337 To: ngraham, #frameworks, #dolphin,

D12337: Give the file dialogs a "Sort by" menu button on the toolbar

ngraham edited the test plan for this revision. REPOSITORY R241 KIO BRANCH arcpatch-D12337 REVISION DETAIL https://phabricator.kde.org/D12337 To: ngraham, #frameworks, #dolphin, #vdg, rkflx Cc: kde-frameworks-devel, andreaska, markg, broulik, anemeth, michaelh, ngraham, bruns

D12337: Give the file dialogs a "Sort by" menu button on the toolbar

ngraham marked 2 inline comments as done. ngraham added inline comments. INLINE COMMENTS > rkflx wrote in kdiroperator.cpp:1888 > I think it is a misconception that toolbar icons represent state. I don't > know of any toolbar in our software where this is the case, so why should > users

D12337: Give the file dialogs a "Sort by" menu button on the toolbar

ngraham updated this revision to Diff 34313. ngraham added a comment. `view-sort-ascending` it is, for now REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D12337?vs=34312=34313 BRANCH arcpatch-D12337 REVISION DETAIL https://phabricator.kde.org/D12337

D12337: Give the file dialogs a "Sort by" menu button on the toolbar

ngraham updated this revision to Diff 34312. ngraham added a comment. Restricted Application added a subscriber: kde-frameworks-devel. Merge master REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D12337?vs=33498=34312 BRANCH arcpatch-D12337 REVISION DETAIL

D12337: Give the file dialogs a "Sort by" menu button on the toolbar

ngraham added a comment. I see your point @rkflx, but I think it's a mistake to use an icon whose appearance suggests that it represents state when in fact it doesn't--that's the kind of thing that causes the misconception. Since ascending alphabetical order is one of the available states,

D12337: Give the file dialogs a "Sort by" menu button on the toolbar

rkflx accepted this revision. rkflx added a comment. This revision is now accepted and ready to land. Thanks, LGTM now. INLINE COMMENTS > ngraham wrote in kdiroperator.cpp:1888 > The problem conceptually is that `view-sort-ascending` is semantically > inaccurate for anything but ascending

D12337: Give the file dialogs a "Sort by" menu button on the toolbar

ngraham added inline comments. INLINE COMMENTS > kdiroperator.cpp:1887 > KActionMenu *sortMenu = new KActionMenu(i18n("Sorting"), this); > +sortMenu->setIcon(QIcon::fromTheme(QStringLiteral("itemize"))); > +sortMenu->setDelayed(false); Before the patch lands, this icon will be

D12337: Give the file dialogs a "Sort by" menu button on the toolbar

ngraham edited the summary of this revision. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D12337 To: ngraham, #frameworks, #dolphin, #vdg, rkflx Cc: andreaska, markg, broulik, anemeth, michaelh, bruns

D12337: Give the file dialogs a "Sort by" menu button on the toolbar

ngraham edited the test plan for this revision. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D12337 To: ngraham, #frameworks, #dolphin, #vdg, rkflx Cc: andreaska, markg, broulik, anemeth, michaelh, bruns

D12337: Give the file dialogs a "Sort by" menu button on the toolbar

ngraham marked 8 inline comments as done. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D12337 To: ngraham, #frameworks, #dolphin, #vdg, rkflx Cc: andreaska, markg, broulik, anemeth, michaelh, bruns

D12337: Give the file dialogs a "Sort by" menu button on the toolbar

ngraham marked 6 inline comments as done. ngraham added inline comments. INLINE COMMENTS > rkflx wrote in kdiroperator.cpp:1888 > While we agreed upon wanting a better icon, until that's done I'd prefer > `view-sort-ascending` instead. For me, `itemize` has no connection to sorting > at all,

D12337: Give the file dialogs a "Sort by" menu button on the toolbar

ngraham updated this revision to Diff 33498. ngraham added a comment. Revert accidental merge, and simplify the single-click menu activation REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D12337?vs=33464=33498 BRANCH arcpatch-D12337 REVISION DETAIL

D12337: Give the file dialogs a "Sort by" menu button on the toolbar

rkflx added inline comments. INLINE COMMENTS > kdiroperator.cpp:1888 > d->actionCollection->addAction(QStringLiteral("sorting menu"), > sortMenu); > +sortMenu->setIcon(QIcon::fromTheme(QStringLiteral("itemize"))); > While we agreed upon wanting a better icon, until that's done I'd

D12337: Give the file dialogs a "Sort by" menu button on the toolbar

ngraham added a comment. Yeah, once all is said and done, we might be able to remove both the View and Sorting menu items from the file dialog's context menu. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D12337 To: ngraham, #frameworks, #dolphin, #vdg, rkflx Cc:

D12337: Give the file dialogs a "Sort by" menu button on the toolbar

ngraham updated this revision to Diff 33464. ngraham added a comment. - Address review comments: Sorting > Sort by Name REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D12337?vs=33459=33464 BRANCH arcpatch-D12337 REVISION DETAIL

D12337: Give the file dialogs a "Sort by" menu button on the toolbar

ngraham edited the test plan for this revision. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D12337 To: ngraham, #frameworks, #dolphin, #vdg, rkflx Cc: andreaska, markg, broulik, anemeth, michaelh, bruns

D12337: Give the file dialogs a "Sort by" menu button on the toolbar

rkflx added a comment. In D12337#256813 , @ngraham wrote: > the context menu will have "Sorting > Sort by name. Thoughts? Actually I'd still find that kind of acceptable. REPOSITORY R241 KIO REVISION DETAIL

D12337: Give the file dialogs a "Sort by" menu button on the toolbar

rkflx added a comment. In D12337#256813 , @ngraham wrote: > Now we have a problem. With no text in the toolbar button, the actual items don't reveal their purpose so readily unless we change all of their strings to say "Sort by Name" (etc.).

D12337: Give the file dialogs a "Sort by" menu button on the toolbar

ngraham added a comment. Now we have a problem. With no text in the toolbar button, the actual items don't reveal their purpose so readily unless we change all of their strings to say "Sort by Name" (etc.). But if we do that, then the context menu will have "Sorting > Sort by name.

D12337: Give the file dialogs a "Sort by" menu button on the toolbar

ngraham edited the summary of this revision. ngraham edited the test plan for this revision. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D12337 To: ngraham, #frameworks, #dolphin, #vdg, rkflx Cc: andreaska, markg, broulik, anemeth, michaelh, bruns

D12337: Give the file dialogs a "Sort by" menu button on the toolbar

ngraham updated this revision to Diff 33459. ngraham added a comment. - Address review comments REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D12337?vs=32533=33459 BRANCH arcpatch-D12337 REVISION DETAIL https://phabricator.kde.org/D12337 AFFECTED FILES

D12337: Give the file dialogs a "Sort by" menu button on the toolbar

ngraham added a comment. Better "sort options" icon - https://bugs.kde.org/show_bug.cgi?id=393318 Bring back arrows in menu toolbuttons - https://bugs.kde.org/show_bug.cgi?id=344746 On the arrow issue, apparently this is actually a design decision and not a bug. If others also feel

D12337: Give the file dialogs a "Sort by" menu button on the toolbar

ngraham planned changes to this revision. ngraham added a comment. You make a lot of good points, Henrik. One of these days I'll learn that whenever you and I disagree, you're probably right! :-) I'll agree to an icon-only button if we can get a better icon and make Breeze display

D12337: Give the file dialogs a "Sort by" menu button on the toolbar

rkflx added a comment. Hm, now you are getting quite off-topic. I thought we wanted to make sorting easier? In D12337#250147 , @ngraham wrote: > Nevertheless, this needs to be a broader discussion, and we can't address it as a part of the

D12337: Give the file dialogs a "Sort by" menu button on the toolbar

ngraham added a comment. If I step back, I think one reason why I find myself objecting to icon-only toolbuttons is because of an in-my-opinion unfortunate confluence of factors that make them difficult to distinguish as interactive UI elements: - No border: doesn't actually look like a

D12337: Give the file dialogs a "Sort by" menu button on the toolbar

ngraham added a comment. In D12337#249988 , @markg wrote: > Well, the icon certainly is better and without text is much better as well, but i still don't like (nor see the value) of having this sort option that prominent "in your face". >

D12337: Give the file dialogs a "Sort by" menu button on the toolbar

markg added a comment. In D12337#249814 , @rkflx wrote: > In D12337#249528 , @rkflx wrote: > > > F5812548: KIO-toolbar-sort-button.png > > > @broulik

D12337: Give the file dialogs a "Sort by" menu button on the toolbar

rkflx added a comment. In D12337#249818 , @ngraham wrote: > In D12337#249817 , @rkflx wrote: > > > @ngraham One more idea: Instead of having the text right in the toolbar, move it as a header inside

D12337: Give the file dialogs a "Sort by" menu button on the toolbar

ngraham added a comment. FWIW, here's the macOS sort icon, as shown in the open dialog: F5813078: 0-macOS open.png I'm not sure it's good enough, and I know that it's not considered very discoverable in the Mac world. REPOSITORY R241 KIO

D12337: Give the file dialogs a "Sort by" menu button on the toolbar

ngraham added a comment. In D12337#249817 , @rkflx wrote: > @ngraham One more idea: Instead of having the text right in the toolbar, move it as a header inside the menu, e.g. like this: > > --- Sort by --- > Name > Size >

D12337: Give the file dialogs a "Sort by" menu button on the toolbar

rkflx added a comment. @ngraham One more idea: Instead of having the text right in the toolbar, move it as a header inside the menu, e.g. like this: --- Sort by --- Name Size ... REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D12337 To:

D12337: Give the file dialogs a "Sort by" menu button on the toolbar

rkflx added a subscriber: andreaska. rkflx added a comment. In D12337#249528 , @rkflx wrote: > F5812548: KIO-toolbar-sort-button.png @broulik @markg Do you like the screenshot above better? The

D12337: Give the file dialogs a "Sort by" menu button on the toolbar

ngraham added a comment. In D12337#249528 , @rkflx wrote: > I know you feel strongly about putting Sort functionality right in the face of users ;) Let me play the devil's advocate to work out whether another solution would be feasible too: >

D12337: Give the file dialogs a "Sort by" menu button on the toolbar

ngraham added a comment. In D12337#249793 , @markg wrote: > In D12337#249784 , @ngraham wrote: > > > In D12337#249782 , @markg wrote: > > > > > Sort

D12337: Give the file dialogs a "Sort by" menu button on the toolbar

markg added a comment. In D12337#249784 , @ngraham wrote: > In D12337#249782 , @markg wrote: > > > Sort already is "right in front" of the user. They merely have to click the column headers. > >

D12337: Give the file dialogs a "Sort by" menu button on the toolbar

ngraham added a comment. In D12337#249782 , @markg wrote: > Sort already is "right in front" of the user. They merely have to click the column headers. ...Only if the view has visible columns. Short View and Tree View don't. REPOSITORY

D12337: Give the file dialogs a "Sort by" menu button on the toolbar

markg added a comment. Sort already is "right in front" of the user. They merely have to click the column headers. These column headers used to (before the breeze theme, i think the air theme had it) show arrows indicating if something was sorted in ascending or descending. I don't know

D12337: Give the file dialogs a "Sort by" menu button on the toolbar

broulik added a comment. I don't like how the button floats in "mid air" between the two sides of the toolbar REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D12337 To: ngraham, #frameworks, #dolphin, #vdg, rkflx Cc: broulik, anemeth, michaelh, bruns

D12337: Give the file dialogs a "Sort by" menu button on the toolbar

rkflx added a comment. I know you feel strongly about putting Sort functionality right in the face of users ;) Let me play the devil's advocate to work out whether another solution would be feasible too: - Introducing a new string to the UI comes at a cost: It draws attention away from

D12337: Give the file dialogs a "Sort by" menu button on the toolbar

ngraham edited the test plan for this revision. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D12337 To: ngraham, #frameworks, #dolphin, #vdg, rkflx Cc: anemeth, michaelh, bruns

D12337: Give the file dialogs a "Sort by" menu button on the toolbar

ngraham added a task: T8552: Polish Open/Save dialogs. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D12337 To: ngraham, #frameworks, #dolphin, #vdg, rkflx Cc: anemeth, michaelh, bruns

D12337: Give the file dialogs a "Sort by" menu button on the toolbar

ngraham edited the summary of this revision. ngraham edited the test plan for this revision. ngraham set the repository for this revision to R241 KIO. ngraham added a dependency: D12333: Put the open/save dialog's toolbar above all other widgets, like Dolphin does. Restricted Application added a

D12337: Give the file dialogs a "Sort by" menu button on the toolbar

ngraham updated this revision to Diff 32533. ngraham added a comment. Improve a comment CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D12337?vs=32532=32533 BRANCH move-sort-order-chooser-to-toolbar (branched from master) REVISION DETAIL https://phabricator.kde.org/D12337

D12337: Give the file dialogs a "Sort by" menu button on the toolbar

ngraham created this revision. ngraham added reviewers: Frameworks, Dolphin, VDG, rkflx. ngraham requested review of this revision. REVISION SUMMARY This patch moves the sort order chooser and options out of the somewhat hidden Settings menu and onto the main toolbar, in conjunction with a few