folderspanel context-menu option "Limit to Home Directory" should be always visible
ClosedPublic

Authored by michaelh on Jan 4 2018, 6:11 PM.

Details

Summary

Only by chance I discovered that this option is visible but only when inside home. Before that I always edited dolphinrc to reenable it.
I think it's less confusing to always show it but toggle its enabled state

Test Plan

compile and run
show folderspanel context-menu in different places

Diff Detail

Repository
R318 Dolphin
Branch
folderpanel (branched from Applications/17.12)
Lint
No Linters Available
Unit
No Unit Test Coverage
michaelh requested review of this revision.Jan 4 2018, 6:11 PM
michaelh created this revision.
michaelh removed a subscriber: Dolphin.

+1, I suppose this is fine.

ngraham requested changes to this revision.Jan 13 2018, 12:15 AM

Doesn't work 100% for me: After I uncheck "Limit to home directory", the menu item becomes permanently disabled and I can't re-enable it.

This revision now requires changes to proceed.Jan 13 2018, 12:15 AM

Even when you right-click home or a folder inside of it?
I re-checked and it still works as expected here.

src/panels/folders/treeviewcontextmenu.cpp
130–136

I have also tested

const bool enableLimitToHomeDirectory = url.isLocalFile();

It looks even more consistent to me. With one exception:
When

  1. current url is outside home
  2. 'Limit to is unchecked

folderpanel stays as is when checking 'Limit to ...'.
The option only becomes effective after navigating inside home.
May be we should jump to home in this case.

I see, the menu item iss only active when you right-click on a path in your home directory. I would support making it always active and jumping you back to your home directory if you're outside it.

Does a protocol (except 'file://') exist that relies on/utilizes the 'Show hidden folders'-option?
I've checked some but didn't find any.
If none exists, 'Show hidden folders'-option should behave same way as 'Limit to...'.

This comment was removed by michaelh.

Cannot find the dolphin API Documentation, specially FoldersPanel class. Where is it?

michaelh updated this revision to Diff 25317.Jan 14 2018, 2:17 PM
  • Introduced optional parameter to FoldersPanel::loadTree()
  • Added navigate home functionality
  • Reordered if.. block
  • Condition to enable 'Limit to...' option
ngraham accepted this revision.Jan 14 2018, 2:57 PM

Much better. Now the menu item is always available, and activating it jumps you to the home directory if you're outside of it. And you even checked for the case where's you're inside a subdirectory of ~.

This revision is now accepted and ready to land.Jan 14 2018, 2:57 PM
michaelh added a comment.EditedJan 14 2018, 3:09 PM

Don't land it yet, please.
I want to document allowJumpHome param first.

What about 'Show hidden folders'?

michaelh updated this revision to Diff 25325.Jan 14 2018, 3:58 PM

Documented allowJumpHome param

michaelh updated this revision to Diff 25328.Jan 14 2018, 4:33 PM

revert more

michaelh updated this revision to Diff 25333.Jan 14 2018, 5:42 PM

Documented allowJumpHome Param

ngraham closed this revision.Jan 14 2018, 6:03 PM

I'm late to the party, but oh well.

src/panels/folders/folderspanel.cpp
330

While at it, use QStringLiteral()

352

if (jump) is enough, no need to compare with true.

src/panels/folders/folderspanel.h
94

Bools in APIs are bad for readability, better use enums (e.g. loadUrl(url, FoldersPanel::AllowJumpHome)).

@michaelh, would you care to prepare another patch to address @elvisangelaccio's comments?

CMakeLists.txt
6 ↗(On Diff #25325)

Don't touch this; the release process manages it.

michaelh marked 2 inline comments as done.Jan 15 2018, 11:01 AM

The new diff is prepared. To be safe I did

$ git pull
$ git checkout -b folderpanel-refac origin/Applications/17.12

(made changes)
git diff looks ok

As this one is already closed. I am reluctant to

arc diff
CMakeLists.txt
6 ↗(On Diff #25325)

After the splendid time we had yesterday sorting out the diff-mess I produced, I can honestly say "I did not do that on purpose. And, Your Honour, I hereby swear I will never change cmake files (except for a good reason)" :)

src/panels/folders/folderspanel.cpp
330

Should this have the same effect as

baseUrl = QUrl::fromLocalFile(QDir::rootPath());

I would like to refactor this if..block a little bit.

src/panels/folders/folderspanel.h
94

Please explain this a little bit. Because by looking at this code I can see no advantage.

michaelh marked 4 inline comments as done.Jan 16 2018, 2:24 PM

Continued here D9911