Differential D26782

KFilePlacesModel: On new profile in recent show only recentlyused:/ based entries by default
ClosedPublic
Actions

Authored by meven on Jan 20 2020, 9:14 AM.

Details

Reviewers
ngraham
hein
Group Reviewers
VDG
Plasma
Commits
R241:3dfa0c1c2f87: KFilePlacesModel: On new profile in recent show only recentlyused:/ based…
Summary

The places timeline:/today and timeline:/yesterday are way less useful than recentlyused:/files and recentlyused/folders.
We should expose only one of the two for new installations, to not clutter UI.

Test Plan

rm ~/.local/share/user-places.xbel

Before:

After:

Diff Detail

Repository
R241 KIO
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
meven created this revision.Jan 20 2020, 9:14 AM
Restricted Application added a project: Frameworks. · View Herald TranscriptJan 20 2020, 9:14 AM
Restricted Application added a subscriber: kde-frameworks-devel. · View Herald Transcript
meven requested review of this revision.Jan 20 2020, 9:14 AM
meven updated this revision to Diff 73917.Jan 20 2020, 9:15 AM

Clean commit

meven edited the summary of this revision. (Show Details)Jan 20 2020, 9:18 AM
meven edited the test plan for this revision. (Show Details)

Too bad I missed the boat for KF 5.66 and Plasma 5.18.

ngraham added a comment.Jan 21 2020, 1:09 AM

Oh, I thought we already did this. +1 since it was explicitly the plan.

But should we just hide them, or not even create them in the first place? Creating hidden-by-default entries seems a bit off.

src/filewidgets/kfileplacesmodel.cpp
356–357

space before the =

meven added a comment.Jan 21 2020, 8:43 AM
In D26782#597811, @ngraham wrote>

But should we just hide them, or not even create them in the first place? Creating hidden-by-default entries seems a bit off.

I chose hidden by value just to keep the feature somewhat more discoverable than if it was not present at all.
I would be fine not adding those in the first place too. It may be simpler also.

meven updated this revision to Diff 73995.Jan 21 2020, 8:46 AM
meven marked an inline comment as done.

Added missing spaces

ngraham added a comment.Jan 21 2020, 4:47 PM
In D26782#597916, @meven wrote:
In D26782#597811, @ngraham wrote>

But should we just hide them, or not even create them in the first place? Creating hidden-by-default entries seems a bit off.

I chose hidden by value just to keep the feature somewhat more discoverable than if it was not present at all.
I would be fine not adding those in the first place too. It may be simpler also.

I would recommend it. Anything that's hidden by default isn't actually discoverable at all, and it sends the wrong message too; it means that we don't actually have confidence in the feature (as indeed, we do not for these old entries). I think it would be simpler and more appropriate to just not add them at all.

meven updated this revision to Diff 74071.Jan 22 2020, 8:06 AM

Don't add by default 'Modifield Today' and 'Modifield Yesterday' when recentlyused:/ is present

meven updated this revision to Diff 74072.Jan 22 2020, 8:07 AM

Clean up

ngraham accepted this revision.Jan 22 2020, 2:38 PM
ngraham added inline comments.
src/filewidgets/kfileplacesmodel.cpp
358

s/Modifield/Modified/

This revision is now accepted and ready to land.Jan 22 2020, 2:38 PM
meven updated this revision to Diff 74123.Jan 22 2020, 2:41 PM

Fix comment

meven marked an inline comment as done.Jan 22 2020, 2:42 PM
Closed by commit R241:3dfa0c1c2f87: KFilePlacesModel: On new profile in recent show only recentlyused:/ based… (authored by meven). · Explain WhyJan 22 2020, 2:46 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Msrc/filewidgets/kfileplacesmodel.cpp (17 lines)

Diff 74124

View Options

src/filewidgets/kfileplacesmodel.cpp

Loading...