[Places panel] Revamp the Recently Saved section
Needs ReviewPublic

Authored by meven on Aug 20 2017, 11:06 PM.

Details

Summary

FEATURE: 110016
FEATURE: 311218
FIXED-IN: 5.62

Revamp the Recently Saved section, including the following changes:

  • Change the section name to "Recent"
  • Change the name of the existing entries to "Modified Today" and "Modified Yesterday", for clarity.
  • Add a "Recently Used" item corresponding to recentlyused:/, and adjust the code to support this.

Depends on D22144

Test Plan

Login in with a new user account or rm ~/.local/share/user-places.xbel. You'll see the new Recently Used place in Dolphin and the file picker:

Existing user accounts are untouched.

Thanks to D14893, the contents of the Recently Used section are actually useful.

Diff Detail

Repository
R241 KIO
Branch
arcpatch-D7446
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 14315
Build 14333: arc lint + arc unit
There are a very large number of changes, so older changes are hidden. Show Older Changes
markg requested changes to this revision.Oct 12 2017, 6:03 PM

That said, it's not just me who wants this in Dolphin, too. There's a Bugzilla ticket with one dupe and 32 votes: https://bugs.kde.org/show_bug.cgi?id=357542

Also, we have a semi-functional version of this already in the form of the Places panel's Recently Saved section. So there does seem to be some interest, both historical and current, in having Dolphin show recent files by default.

Be aware that the thing you're trying to get approved here is not the fix for that bug!
You'd have to dig in Baloo to fix that bug.

You should not even be looking into the recentdocuments KIO slave as that is just not the part where the bug is (in this case).
Also, the users' case is for the file open/save dialog, again not the main application.

You yourself even said in the bug that this patch would only be a non optimal workaround. This is imho the most nasty thing; patches that "seem" to solve something but are in fact workarounds that merely hide the real problem. I admit, I - unknowingly - also did that in the past but was often corrected by someone. As you are now :)

So, to make things clear.
-1 for having this in dolphin' main window. Doesn't mean that it never gets in, just that i don't like it.
+1 for having this on a contextual basis in the file/folder open/save dialogs, that would in fact be very beneficial! You should still look at Baloo for fixing it, not recentdocuments.

This revision now requires changes to proceed.Oct 12 2017, 6:03 PM

You can add recent documents as an action like in Kate. Places model looks in not correct destination.

You can add recent documents as an action like in Kate. Places model looks in not correct destination.

I'm not sure exactly what this is referring to. Can you clarify?

I support having the recently used option, perhaps by default in the open/save dialogue, and also as an optional panel.
This would be highly useful for workflow for many, including using other software as suggested, but also coming back to documents (in multiple locations) being referred to (though not necessarily saved, so not appearing in the 'recently saved' section.

(It's unfortunate that there's not a user-friendly way of selecting components of the Places panel).

ngraham updated this revision to Diff 42277.Sep 25 2018, 2:38 AM

Re-base on master

Restricted Application edited subscribers, added: kde-frameworks-devel; removed: Frameworks. · View Herald TranscriptSep 25 2018, 2:38 AM
ngraham retitled this revision from Add a Recent Documents places item to Dolphin and file pickers by default to [Places panel] Add a Recently Used item by default.Sep 25 2018, 2:40 AM
ngraham edited the summary of this revision. (Show Details)
ngraham edited the test plan for this revision. (Show Details)

I'd like to re-submit this patch (now re-based on master) for consideration. I put some work into improving the recentdocuments:/ ioslave in D14893 so it's useful now. User requests for this feature to be added by default still pop up from time to time, and both GNOME and macOS both have this feature in their file browsers and file pickers. I think it's relevant and desirable.

While I wouldn't use it (yet?) I think it's a great addition and would benefit a lot of people. I give it a +1, especially if D15739 gets approved aswell. ;)

ngraham edited the summary of this revision. (Show Details)Oct 3 2018, 11:29 PM

This is in frameworks, recentdocuments is in kio-extras.

Are you sure you can do this?

This is in frameworks, recentdocuments is in kio-extras.

Are you sure you can do this?

Not sure I catch your meaning. The recentdocuments:/ kioslave has been shipped since forever; this patch just adds it to the Places panel by default.

dfaure added a comment.Oct 4 2018, 2:19 AM

I guess the implicit part of the question is "what if kio-extras isn't installed ?".
I assume it will lead to a broken item.

So this should probably use a check like KProtocolInfo::isKnownProtocol("recentdocuments")

I guess the implicit part of the question is "what if kio-extras isn't installed ?".
I assume it will lead to a broken item.

So this should probably use a check like KProtocolInfo::isKnownProtocol("recentdocuments")

Ah, I see now. Sure, I can do that.

ngraham updated this revision to Diff 42865.Oct 4 2018, 4:24 PM

Only create "Recently Used" if the protocol is available because kio-extras is installed

huftis added a subscriber: huftis.Nov 4 2018, 7:25 PM
ngraham updated this revision to Diff 45040.Nov 7 2018, 3:17 PM
  • Revamp the whole section

I would suggest to have by default detail view instead of icon view cause with detail view it's easier to read the paths.

ngraham updated this revision to Diff 45043.Nov 7 2018, 3:32 PM

Rebase on master

Updating D7446: [Places panel] Add a Recently Used item by default

ngraham retitled this revision from [Places panel] Add a Recently Used item by default to [Places panel] Revamp the Recently Saved section.Nov 7 2018, 3:34 PM
ngraham edited the summary of this revision. (Show Details)
ngraham edited the test plan for this revision. (Show Details)
ngraham added a comment.EditedNov 7 2018, 3:42 PM

I would suggest to have by default detail view instead of icon view cause with detail view it's easier to read the paths.

The patch actually has the happy side effect of doing this. :)

I like this change a lot, however I think the Section header should be changed from "Recents" to "Recently Used". I think Recents just sound inelegant and weird

Using "Recently Used" as the section header label presents the following problems:

  • One of the items is named "Recently Used"
  • The other items actually do not display a list of recently used files; they display a list of recently saved files.

That's why I was trying to come up with a generic section header label that could feasible encompass both.

Note also that Kickoff's "History" with recent files is /yet another/ completely different data source.

elvisangelaccio requested changes to this revision.Dec 2 2018, 10:08 AM

We need to update kfileplacesmodeltest (as well as placesitemmodeltest in dolphin).

This revision now requires changes to proceed.Dec 2 2018, 10:08 AM
ngraham edited the summary of this revision. (Show Details)Jan 21 2019, 11:44 PM
meven added a subscriber: meven.Jun 23 2019, 9:26 PM

Using "Recently Used" as the section header label presents the following problems:

  • One of the items is named "Recently Used"
  • The other items actually do not display a list of recently used files; they display a list of recently saved files.

    That's why I was trying to come up with a generic I volunteer myself to update the tests.section header label that could feasible encompass both.

I think we should use another word than "Recents" like "Recent" singular, "Recent files", "Recent Activity" or event "Timeline" perhaps but "Recent" has my preference.
It is a category title usually they are nouns. We can use an adjective but since a plural form is weird not to have a noun coming with it.

I volunteer myself to update the tests.

"Recent" sounds fine to me. Feel free to commandeer this revision if you'd like to finish it up and handle the tests. These places panel tests are the bane of my existence! :)

meven commandeered this revision.Jun 23 2019, 10:30 PM
meven added a reviewer: ngraham.
meven added inline comments.
src/filewidgets/kfileplacesmodel.cpp
335

timeline protocol works on modification date, we should change this to "Modified Today".
It is a better feature anyway.

338

same here

Now that we have creation date support, what do you think about adjusting the timeline to show files that were either created or modified during a given time period?

Now that we have creation date support, what do you think about adjusting the timeline to show files that were either created or modified during a given time period?

Well modified files already includes created files, so that is pretty much accomplished.
And baloo probably does not support file date creation metadata.

src/filewidgets/kfileplacesitem.cpp
113

Rename to "Recent"

After some discussion, we realized we have three different sets of "recent documents" backends :

  • recentdocument ioslave using KDirWatch that miss accessed dir and files (used in places)
  • baloo's timeline ioslave using baloo extractor, that requires baloo but has indexing and filtering capatibilities (used in places)
  • kactivities-stats that has activity context, and works for modified or accessed files, but that has no ioslave (used in kickoff and kicker)

It appears kactivities-stats has the best feature set compare to recentdocuments.

So my plan is to create a kacitity-stats "recentdocuments" ioslave and overwrite current recentdocumenst ioslave with it.
And then used this primarly in dolphin/kfilewidgets with baloo still optionally around for its advanced features.

CC @davidedmundson @kbroulik

meven planned changes to this revision.Jun 25 2019, 8:41 PM

I have started D22082 enrolling the plan set in the previous comment.

Once this task has done enough progress and has landed some of its work, I will be able to revisit this diff.

meven added a comment.Jun 30 2019, 9:11 PM
In D7446#486700, @meven wrote:

I have started D22082 enrolling the plan set in the previous comment.

Once this task has done enough progress and has landed some of its work, I will be able to revisit this diff.

Work has moved to the kio-extras repo and is good enough for review D22144

meven requested review of this revision.Jul 23 2019, 9:31 AM
meven updated this revision to Diff 62380.EditedJul 23 2019, 9:36 AM

Use recentlyused:/ ioslave introduced in D22144

Results in :

meven marked 3 inline comments as done.Jul 23 2019, 9:38 AM

With your awesome new recentlyused:/ ioslave, I find myself wondering if the old timeline:/ ioslaves even still have value, and if it makes sense to have them in the Places Panel by default.

meven added a comment.Jul 23 2019, 3:31 PM

With your awesome new recentlyused:/ ioslave, I find myself wondering if the old timeline:/ ioslaves even still have value, and if it makes sense to have them in the Places Panel by default.

The today and yesterday, date filters timeline:/ offers are features I would like to add to KactivitiesStats.
But in the meantime we don't have the same features offered so IMO best keep it there for now, so that when users have baloo enabled they don't loose this feature.

But I agree, the end goal would be to hide timeline:/ in default settings.

meven edited the summary of this revision. (Show Details)Jul 23 2019, 3:57 PM
meven edited the test plan for this revision. (Show Details)
ngraham accepted this revision.Jul 23 2019, 4:32 PM
meven added a comment.Mon, Jul 29, 7:33 AM

I am working on allow to filter by resource activity date D22717 and D22775 (I will add this soonish into recentlyused:/ ioslave D22144)

This will allow us to provide similar filters as timeline:/ has : accessed today, accessed yesterday...
And perhaps replace timeline:/ default entries in places, at least when baloo is turned off.

And since I am also adding date range filtering, I ask what date range would we want to have ?

My proposition would be: today, yesterday, this week (today and the previous 6 days) and maybe this month.
No more than 4, for sure, maybe three would be enough.
But I am all ears for inputs.

In D7446#503575, @meven wrote:

I am working on allow to filter by resource activity date D22717 and D22775 (I will add this soonish into recentlyused:/ ioslave D22144)

This will allow us to provide similar filters as timeline:/ has : accessed today, accessed yesterday...
And perhaps replace timeline:/ default entries in places, at least when baloo is turned off.

And since I am also adding date range filtering, I ask what date range would we want to have ?

My proposition would be: today, yesterday, this week (today and the previous 6 days) and maybe this month.
No more than 4, for sure, maybe three would be enough.
But I am all ears for inputs.

Right now by default we only show two: "today" and "last month". We should probably keep it at that, at most; my sense is that even these are very infrequently used. Sometimes I wonder if they should even be on the placed panel by default at all. Especially once your new recentlyused:/ ioslave is merged, it's so good that it will obviate the need for the "today" one, at the minimum.

meven added a comment.Mon, Aug 12, 8:08 AM
In D7446#505870, @ngraham wrote:ut I am all ears for inputs.

Right now by default we only show two: "today" and "last month". We should probably keep it at that, at most; my sense is that even these are very infrequently used. Sometimes I wonder if they should even be on the placed panel by default at all. Especially once your new recentlyused:/ ioslave is merged, it's so good that it will obviate the need for the "today" one, at the minimum.

My proposal was about which default entries to have for the recentlyused:/ kio slave.
With D22717 we have now date filtering and with D22775 we will be able to have date range filtering in recentlyused:/ and hence we can ask ourselves which are the default entries to expose.

meven edited the summary of this revision. (Show Details)Sat, Aug 17, 5:13 PM
meven edited the summary of this revision. (Show Details)