Add 'Sort By' and 'View Mode' into Dolphin file context menus
ClosedPublic

Authored by nerdopolist on May 15 2018, 11:30 PM.

Details

Summary

This adds the 'Sort By' and 'View Mode' options into the context menu of Dolphin within the file browser. I keep looking for these option (especially sort by) in the Context Menu, and keep forgetting to go to the menu. It also makes the order of "View Mode" and "Sort By" options consistent in the control menu

Test Plan

Made sure that the options appeared when right clicking on an empty space.

Diff Detail

Repository
R318 Dolphin
Lint
Lint Skipped
Unit
Unit Tests Skipped
nerdopolist created this revision.May 15 2018, 11:30 PM
Restricted Application added a subscriber: kfm-devel. · View Herald TranscriptMay 15 2018, 11:30 PM
nerdopolist requested review of this revision.May 15 2018, 11:30 PM

Can we move these down one section, below Rename and Move to Trash?

rkflx added a subscriber: rkflx.May 16 2018, 7:09 AM

As there are already quite a lot of menu entries (in particular for context menus of items themselves), do we really need View mode if it is readily available in the toolbar?

(Also, with the planned changes to the file dialog, another idea would be to add a sort button to the toolbar instead. I'm not saying that's what you should do, I'm only mentioning this for consistency.)

I agree with Henrik and think this should not be part of the context menu and then it should not appear when clicking on files or folders but only when clicking on empty space.

Only show the 'Sort By' and 'View Mode' options when right clicking on whitespace, and not on files and folders

ngraham accepted this revision.May 16 2018, 6:04 PM

As there are already quite a lot of menu entries (in particular for context menus of items themselves), do we really need View mode if it is readily available in the toolbar?

(Also, with the planned changes to the file dialog, another idea would be to add a sort button to the toolbar instead. I'm not saying that's what you should do, I'm only mentioning this for consistency.)

Then again, the file dialog's context menu does have the sort and view mode items in it, so it would actually be consistent to add it here! :) I don't think we're planning to remove it from the context menu (at least not in my patch D12337: Give the file dialogs a "Sort by" menu button on the toolbar).

With this only affecting whitespace now, I'm okay with it.

This revision is now accepted and ready to land.May 16 2018, 6:04 PM

Let's see what other Dolphin folks think before landing it, though.

Hmm, 'Sort By' could make sense, but why also 'View Mode'? What's wrong with the toolbar buttons?

People could remove the toolbar buttons, I guess. It might be nice to have this available in the context menu in that case.

markg added a subscriber: markg.May 17 2018, 6:58 PM

Hmm, 'Sort By' could make sense, but why also 'View Mode'? What's wrong with the toolbar buttons?

I didn't want to say it as i would be negative again about a feature request ;)
I had the exact same idea, sort mode isn't in the toolbar by default (it is there to add though).
View mode is and is even less clicks then adding it under the right mouse button.

But things are going to change.
The sort mode is going to be in the next dolphin toolbar by default (right?) so the toolbar then has view and sort. Right there with less clicks then the RMB would have been.

I can't imagine users are already used to this as:
Thunar: doesn't have either in the RMB
Files (gnome): doesn't have either
Windows explorer: doesn't have it (i think, had to search for screenshots as i don't have windows anymore)
Does macOS' finder have it?

In summary, nobody has it. The RMB is imho full enough as is already so lets not add these two.
If people consciously remove the view and sort mode from the toolbar then it's their decision. They apparently want to have that. We should not have to add more menu items for that reason. Just don't remove it from the toolbar if you want to have easy access.

As you can see, no +1 or -1 from me :) The above is just how i think about it, just a opinion.

rkflx added a comment.May 17 2018, 7:40 PM

I already voiced my opinion, but I'm also in favour of arguing with facts on not assumptions ;)

But things are going to change.
The sort mode is going to be in the next dolphin toolbar by default (right?) so the toolbar then has view and sort. Right there with less clicks then the RMB would have been.

Source? Currently this is planned for the file dialog, applying the same change for Dolphin would be possible (but needs a separate discussion).

Windows explorer: doesn't have it (i think, had to search for screenshots as i don't have windows anymore)

Windows Explorer displays both entries when clicking on an empty area:

Does macOS' finder have it?

Same thing here:


Given that, the current state of the patch is probably okay (since both entries are no longer displayed when right-clicking on a file). The question now is whether the space in the context menu is best given to those options, or rather to some other (future) entries.

rkflx added a comment.May 17 2018, 8:04 PM

@nerdopolist Please submit your Diff with Arcanist so it includes authorship information (we've been through this before in your other patches).

Also, you should update your test plan to the current state of your patch ;)


@ngraham Any opinion on the position in the context menu now, in particular if Paste is active?

I don't have a very strong opinion on the matter, but it does look a bit odd having them right above the Paste item (when it's active). I almost wonder if these would make sense to have at the very bottom of the menu, after Properties.

I already voiced my opinion, but I'm also in favour of arguing with facts on not assumptions ;)

But things are going to change.
The sort mode is going to be in the next dolphin toolbar by default (right?) so the toolbar then has view and sort. Right there with less clicks then the RMB would have been.

Source? Currently this is planned for the file dialog, applying the same change for Dolphin would be possible (but needs a separate discussion).

That was an assumption (the "right?" gave that away) because it's going to be in the dialog.
Therefore i'm assuming it will (eventually) find it's way into dolphin itself as well as that would be rather consistent.

Windows explorer: doesn't have it (i think, had to search for screenshots as i don't have windows anymore)

Windows Explorer displays both entries when clicking on an empty area:

Does macOS' finder have it?

Same thing here:


Given that, the current state of the patch is probably okay (since both entries are no longer displayed when right-clicking on a file). The question now is whether the space in the context menu is best given to those options, or rather to some other (future) entries.

Ha, great. The two i can't test have it :) I stand corrected.

Thank you for clearing up my assumptions.

nerdopolist added a comment.EditedMay 18 2018, 12:34 AM

I don't have a very strong opinion on the matter, but it does look a bit odd having them right above the Paste item (when it's active). I almost wonder if these would make sense to have at the very bottom of the menu, after Properties.

I wasn't sure where to put it, I did feel like it should be near "Open in New Tab", "Open in New Window", to make sense. But let me know if it should be moved

All right, how about under Paste? I think it looks and feels reasonably at home there:

Move below paste

Rebase with arc

ngraham accepted this revision.May 18 2018, 3:50 AM

Looks good to me. Are other folks okay with it?

Re-visiting the screenshot I posted, it seems like a bit more organizational work needs to be done to re-order items, consolidate sections, and maybe remove some of those separators. If and when this lands, I can work on that a bit.

Also, you should update your test plan to the current state of your patch ;)

@nerdopolist Please don't forget to update your test plan.

nerdopolist edited the test plan for this revision. (Show Details)May 18 2018, 11:22 AM

Also, you should update your test plan to the current state of your patch ;)

@nerdopolist Please don't forget to update your test plan.

Ah. Right. I have updated it now

Ok, I think we can go with this feature, but we should either use the same order we have in the Control menu (View Mode first, Sort By 2nd) or swap the existing entries in the Control menu.

Given that the file dialog has Sorting before View, we should probably go with the latter.

Great! So @nerdopolist, please also swap the order of the VIew Mode and Sort by entries in the Control menu and then this should be good to go.

nerdopolist edited the summary of this revision. (Show Details)

Switch order of "View Mode" and "Sort By" in the Dolphin control menu

nerdopolist edited the summary of this revision. (Show Details)

Rebase, Switch order of "View Mode" and "Sort By" in the Dolphin control menu

Thanks for your flexibility and patience, so I hate to do this to you...

But now, the ordering doesn't match in the View menu! Would you mind reversing them there too? That way we'll have Sort By before View mode in all three places.

Once that's done, I think we'll finally be ready to land this.

rkflx added a comment.May 21 2018, 6:24 AM

Move below paste

Seems like you lost that change.

Move below paste

Seems like you lost that change.

Dang, I did. I got messed up between the repo in my chroot, and out of my chroot. This will be fixed in the one coming right now

Also change the order of "Sort" and "View By" in the "View" menu

@question for @elvisangelaccio or other more experienced Dolphin folks: my impression was that in order for the dolphinui.rc change to take effect for existing installs, the version number at the top of the file needed to be bumped. I tried doing that but it didn't seem to take effect, not even after blowing away dolphinrc and logging out and back in again. Works fine for a new user account though. Any idea what could be going on here?

elvisangelaccio requested changes to this revision.May 26 2018, 3:21 PM

@question for @elvisangelaccio or other more experienced Dolphin folks: my impression was that in order for the dolphinui.rc change to take effect for existing installs, the version number at the top of the file needed to be bumped. I tried doing that but it didn't seem to take effect, not even after blowing away dolphinrc and logging out and back in again. Works fine for a new user account though. Any idea what could be going on here?

Worked for me after bumping the version in dolphinui.rc. Check whether you have a stale ~/.local/share/dolphinui.rc

src/dolphinui.rc
2 ↗(On Diff #34619)

Please bump the version to 21.

This revision now requires changes to proceed.May 26 2018, 3:21 PM

Update for requested changes

elvisangelaccio accepted this revision.Jun 2 2018, 1:25 PM
This revision is now accepted and ready to land.Jun 2 2018, 1:25 PM

@nerdopolist: why does arc create a commit with author "nerdopolis <rbos@rbos>" ?

I see you've been using "Nerdopolis Turfwalker <bluescreenavenger@gmail.com>" in other kwin patches. What should we use to land this patch?

Can you use "Nerdopolis Turfwalker <bluescreenavenger@gmail.com>"
Or do I need to resubmit? My build scripts set that other one as a place holder, and I forgot that that was still in the config...

No need to resubmit, I'll amend the commit locally for this time.

This revision was automatically updated to reflect the committed changes.