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

Authored by ngraham on Apr 19 2018, 4:15 AM.

Details

Summary

This patch moves the sort order chooser and options out of the somewhat hidden Settings menu and onto the main toolbar.

Nice-to-haves that would make this more user-friendly in the future:

Test Plan

New toolbar sorting menu:

It's still there in the context menu and fully accessible for KDirOperator clients, but it's no longer in the file dialog's settings menu.

Diff Detail

Branch
move-sort-order-chooser-to-toolbar (branched from master)
Lint
No Linters Available
Unit
No Unit Test Coverage
ngraham requested review of this revision.Apr 19 2018, 4:15 AM
ngraham created this revision.
ngraham updated this revision to Diff 32533.Apr 19 2018, 4:19 AM

Improve a comment

ngraham edited the summary of this revision. (Show Details)Apr 19 2018, 4:21 AM
ngraham edited the test plan for this revision. (Show Details)
ngraham set the repository for this revision to R241 KIO.
Restricted Application added a project: Frameworks. · View Herald TranscriptApr 19 2018, 4:21 AM
ngraham edited the test plan for this revision. (Show Details)Apr 19 2018, 4:24 AM
rkflx added a comment.Apr 19 2018, 7:47 AM

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 what's really important, i.e. the files. I'm not saying sorting is not important, but is it really that important like you claim?
  • Why would only a single icon get text added next to it?
  • The text may be partly motivated by the lack of an appropriate icon. Shouldn't we improve the icons instead?
  • This conflicts with Detailed View, where some of the options are more easily accessible by clicking on the table headers. Isn't this confusing for users?

Suggestion for consideration:

  • Add well-known icon to Breeze.
  • Put only an icon in the toolbar, but still in a quite central place so it's within easy reach (as opposed to on the very right).
  • Solve the problem where toolbar menu buttons don't get a downwards pointing arrow with Breeze.

I'd prefer this option, but let's hear what others have to say.

I don't like how the button floats in "mid air" between the two sides of the toolbar

markg added a subscriber: markg.Apr 19 2018, 2:58 PM

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 why that's gone but that was the universal way to immediately know which column was sorted and in which order. No need to add a button for that.

If the view needs to stay as clean as it is now then you could only show the arrows in the headers when you hover the header (or view) or when it has focus and let them disappear when it loses focus.

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.

markg added a comment.Apr 19 2018, 3:04 PM

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.

Right, then it should be view dependent.
For the views you posted screenshots of (detailed view) the button would be a waste of space.
To me this sounds more useful as a right mouse button menu (Sort by) on views that don't have header names. In a tree view that also allows for fine grained sorting control (aka, only sort the tree node you clicked on)

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.

Right, then it should be view dependent.
For the views you posted screenshots of (detailed view) the button would be a waste of space.
To me this sounds more useful as a right mouse button menu (Sort by) on views that don't have header names. In a tree view that also allows for fine grained sorting control (aka, only sort the tree node you clicked on)

It's already in the context menu. Context menus are expert features, though. Making it view-dependent is an interesting idea, but I'd worry that having it disappear when Detailed Tree View is being used would give users the idea that said view isn't sortable.

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 what's really important, i.e. the files. I'm not saying sorting is not important, but is it really that important like you claim?
  • Why would only a single icon get text added next to it?
  • The text may be partly motivated by the lack of an appropriate icon. Shouldn't we improve the icons instead?
  • This conflicts with Detailed View, where some of the options are more easily accessible by clicking on the table headers. Isn't this confusing for users?

Being able to easily change the sort order between alphabetical and date modified is a critically important feature. Those are the two most frequently used and most commonly useful sort orders. Right now, for Detailed Tree View, this takes one click: good. But for Short View, it's quite hidden: you need to right-click in the view, or click on the settings icon in the corner that evidence suggests people don't use. And both require the at least-intermediate mousing skill of being able to use a sub-menu.

Adding text to the button is motivated by the lack of a good icon, that's true. But I'm not sure this is a surmountable problem. I don't think I've ever seen an icon that adequately communicated "sorting options here!" all by itself. Have you? An icon-only button would not be sufficient here unless we can find a perfect icon.

  • Solve the problem where toolbar menu buttons don't get a downwards pointing arrow with Breeze.

Definitely. That's been on my to-do list for a while. Currently, they get an arrow only if the button is set up to show a menu on a click-and-hold, but not on a regular click. That's just a regular old bug IMHO.

@broulik @markg Do you like the screenshot above better? The button does not float in the middle, and takes less space.

@andreaska Any tips for a better sort icon? Would it be too much to ask for a new one? (Our current icons often contain characters or a sorting direction, while here we'd need a more generic icon describing a menu containing various sorting options.)

rkflx added a comment.Apr 19 2018, 3:40 PM

@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
  ...

@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
  ...

If we want to emphasize the settings menu, I think we need to make it a lot more prominent and discoverable. I'd be on board with something like this if we gave it text:

[sliders icon] More settings

Or something like that.

Also, we really need to fix the look of headers in Breeze menus...

FWIW, here's the macOS sort icon, as shown in the open dialog:

I'm not sure it's good enough, and I know that it's not considered very discoverable in the Mac world.

rkflx added a comment.Apr 19 2018, 3:57 PM

@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
  ...

If we want to emphasize the settings menu, I think we need to make it a lot more prominent and discoverable. I'd be on board with something like this if we gave it text:

[sliders icon] More settings

Ah, I was not talking about the settings menu. I wanted to show a text-less button, and have the "Sort By" text only visible inside the menu of this separate sort button.

markg added a comment.Apr 19 2018, 7:20 PM

@broulik @markg Do you like the screenshot above better? The button does not float in the middle, and takes less space.

@andreaska Any tips for a better sort icon? Would it be too much to ask for a new one? (Our current icons often contain characters or a sorting direction, while here we'd need a more generic icon describing a menu containing various sorting options.)

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".
You can sort right now via:

  • RMB on the list -> Sorting -> take your pick
  • Settings icon -> Sorting -> take your pick
  • one-click sorting on the headers (if you open a view with headers)

You think we need a fourth option?

I did also just notice that the arrow icon in a header is visible when the view is sorted on that column.
On top of that, the default view (when you don't change anything as a user) is a the detailed view (thus with a visible sort indicator) and one-click sorting on the headers.
You can't satisfy all users, in "my" opinion the current way (without this patch) is fine.

Just because the space is there now doesn't mean the it should be filled with icons ;)

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".
You can sort right now via:

  • Settings icon -> Sorting -> take your pick

This patch removes that.

  • RMB on the list -> Sorting -> take your pick

I can remove it from there too if you'd like :-) (only for the file dialog obviously, not for all KDirOperator clients)

  • one-click sorting on the headers (if you open a view with headers)

Again, only if the view shows column headers.

I did also just notice that the arrow icon in a header is visible when the view is sorted on that column.

If even you--an expert software developer--only just noticed that now, I think that's a fairly good confirmation that it's not a very discoverable feature. :-)

On top of that, the default view (when you don't change anything as a user) is a the detailed view

Not yet; the default view is still Short View, which doesn't show column headers. Even if we change it (in D12327), we will still have one of the two primarily-featured modes lacking headers and therefore an easilt discoverable way to change the sort order.

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 button unless hovered over
  • No text: often difficult to distinguish meaning
  • Minimalistic monochrome line-art icons in Breeze theme: often doesn't scream "I'm a clickable icon!" and can resemble decorations instead--or even worse, mere visual noise
  • No downward-pointing arrows on menu buttons

IMHO, this is a classic case of an "excess of minimalism," where too much symbolism and meaning has been drained away in an effort to distill something to its basic essence. The effort was successful, but went too far!

Icon-only toolbuttons IMHO only really work where they use icons that are truly universally recognizable. If we can't find such an icon, I think it's hard to make the case that an icon-only toolbutton is appropriate.

Nevertheless, this needs to be a broader discussion, and we can't address it as a part of the open/save dialog initiative. Because of this, we need to work around some of the limitations it imposes on us IMHO.

Hm, now you are getting quite off-topic. I thought we wanted to make sorting easier?

Nevertheless, this needs to be a broader discussion, and we can't address it as a part of the open/save dialog initiative. Because of this, we need to work around some of the limitations it imposes on us IMHO.

On the contrary: Because of this, the sorting button should blend into the existing style, until the broader discussion reached an agreement. Adding workarounds everywhere where you push your personal opinion causes inconsistency and problems down the road when issues are fixed at their proper place.

Your points are generic, you don't answer the question why it is suddenly the sorting button where all issues ("perfect" icon, text, button-like look etc.) have to be fixed immediately. You may not agree with some styling choices, but bringing the topic up in every review can get slightly tiring for everyone involved. Fix it in Breeze, instead, if you must.

It's just a sorting button, and even macOS does not display text for it or has a perfect icon. To get what you want you even have to move all buttons to the side to create space, creating more problems…

While you are discussing about the ideal button, people are objecting to the very idea to have this button. I'd rather go for the compromise here, than in the end to have no button at all. Personally I don't use sorting often and know of nobody who does, nevertheless I'd agree to the experiment of making it available more broadly. This does not mean sorting has to be the literal centerpiece of the dialog, though.

ngraham planned changes to this revision.Apr 20 2018, 2:12 AM

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 downward-pointing arrows on menu buttons.

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 that this should be revisited, I would appreciate some support in the matter.

ngraham updated this revision to Diff 33459.May 1 2018, 9:14 PM
  • Address review comments
ngraham edited the summary of this revision. (Show Details)May 1 2018, 9:19 PM
ngraham edited the test plan for this revision. (Show Details)

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 "SortingSort by name. Thoughts?

The issue is that the strings for the submenu items are intimately tied to the text of the parent menu item or toolbar button. Opposition to a toolbar button with a label leads the submenu items being labeled wrong wrong for one of those two UIs.

rkflx added a comment.May 1 2018, 9:41 PM

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 "SortingSort by name. Thoughts?

What about injecting a header as proposed in D12337#249821?

Or could we simply remove the context menu (only for the file dialog)?

rkflx added a comment.May 1 2018, 9:44 PM

the context menu will have "SortingSort by name. Thoughts?

Actually I'd still find that kind of acceptable.

ngraham edited the test plan for this revision. (Show Details)May 1 2018, 10:02 PM
ngraham updated this revision to Diff 33464.May 1 2018, 10:03 PM
  • Address review comments: SortingSort by Name

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.

rkflx added inline comments.May 2 2018, 10:30 AM
src/filewidgets/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, sorry.

I'm aware my alternative shows a specific mode, but TBH I don't think users will be put off too much by this detail, in particular because it is the only sorting-related icon in the dialog.

Anyway, that's just my preference. Let me know if you think itemize is vastly better.

src/filewidgets/kfilewidget.cpp
364–368

?

365

?

558–563

This also worked for me, and would avoid the foreach:

KActionMenu *x = new KActionMenu(QIcon::fromTheme(QStringLiteral("configure")), i18n("Options"), this);
x->setMenu(coll->action(QStringLiteral("sorting menu"))->menu());
x->setDelayed(false);
d->toolbar->addAction(x);
565

Unintentional newline?

1414

?

ngraham updated this revision to Diff 33498.May 2 2018, 2:50 PM

Revert accidental merge, and simplify the single-click menu activation

ngraham marked 6 inline comments as done.May 2 2018, 2:50 PM
ngraham added inline comments.
src/filewidgets/kdiroperator.cpp
1888

The problem conceptually is that view-sort-ascending is semantically inaccurate for anything but ascending order. We don't actually have an icon yet that means "general sort options are here!" That's covered by https://bugs.kde.org/show_bug.cgi?id=393318, and I've pinged Andreas again. No matter what flawed we choose as a placeholder, I'm going to wait for the better icon before landing this, so for now let's just leave it the way it is.

src/filewidgets/kfilewidget.cpp
364–368

Same, sorry.

365

Oops, somehow arc merged this patch with D12333 when I downloaded it. Not sure how I managed to do that. Cleaned up now.

558–563

Found an even simpler way. :)

ngraham marked 8 inline comments as done.May 2 2018, 2:51 PM
ngraham edited the test plan for this revision. (Show Details)
ngraham edited the summary of this revision. (Show Details)May 2 2018, 2:55 PM
ngraham added inline comments.May 2 2018, 2:58 PM
src/filewidgets/kdiroperator.cpp
1887

Before the patch lands, this icon will be changed to whatever https://bugs.kde.org/show_bug.cgi?id=393318 yields.

rkflx accepted this revision.May 2 2018, 9:00 PM

Thanks, LGTM now.

src/filewidgets/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 suddenly expect that what's on the icon represents exactly what is happening, e.g. A-Z ascending? It is merely an example of what type of actions they can expect when clicking on the button.

Icons are a symbolic representation of general concept, not a literal display of a specific state.


Anyway, if you want block everything on that, that's what we'll do.

This revision is now accepted and ready to land.May 2 2018, 9:00 PM

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, using an icon that with "ascending" and "alphabetical" iconography is bound to confuse and frustrate a few people who mistakenly expect it to reflect the active sort order.

Either way, since this contains a string change, it's headed for 5.47 anyway (thanks for being a stickler and reminding me to pay attention to this), so I think we have some time to get a better icon.

ngraham updated this revision to Diff 34312.May 16 2018, 6:28 PM

Merge master

Restricted Application added a subscriber: kde-frameworks-devel. · View Herald TranscriptMay 16 2018, 6:28 PM
ngraham updated this revision to Diff 34313.May 16 2018, 6:33 PM

view-sort-ascending it is, for now

ngraham marked 2 inline comments as done.May 16 2018, 6:36 PM
ngraham added inline comments.
src/filewidgets/kdiroperator.cpp
1888

Seems like the new icon may be a long time in coming, and after giving it more thought, I've come around to your position (an increasingly common occurrence! :) ).

ngraham edited the test plan for this revision. (Show Details)May 16 2018, 6:37 PM
ngraham marked an inline comment as done.

Any opinions from Frameworks folks?

ngraham edited the summary of this revision. (Show Details)May 16 2018, 7:34 PM
ngraham edited the test plan for this revision. (Show Details)

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, e.g.

…and finally present the final outcome to a wider audience for approval before merging. This way can we make progress as time allows, without shipping incomplete features or get bogged down with dependencies. Note I'm not proposing to wait for everything you put into T8552.

(Sorry for not being able to help more currently, but I first wanna reduce my Gwenview and Spectacle backlogs…)

ngraham closed this revision.May 20 2018, 11:58 PM

Done. I've created the file-dialog-improvements branch and landed this there.

ngraham edited the test plan for this revision. (Show Details)Nov 18 2018, 3:07 AM