-
-
Notifications
You must be signed in to change notification settings - Fork 2.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Code Quality: Introduced IWindowsRecentItemsService #16150
Code Quality: Introduced IWindowsRecentItemsService #16150
Conversation
d424572
to
c861720
Compare
This comment was marked as resolved.
This comment was marked as resolved.
Can we use the context menu verb for that? |
0d0a836
to
cf808bc
Compare
src/Files.App/ViewModels/UserControls/Widgets/RecentFilesWidgetViewModel.cs
Show resolved
Hide resolved
It looks like folders are showing in the recent files widget |
This comment was marked as resolved.
This comment was marked as resolved.
Pinned files should be included |
ddf1c68
to
24948f8
Compare
ba20eeb
to
24f5ba6
Compare
This comment was marked as outdated.
This comment was marked as outdated.
There is one issue I discovered where the order of the recent items doesn't match the behavior in |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry to have forgot to mention this, I meant to. |
The behavior in |
I only slightly remember this but I figured it would be reasonable to include why we didn't sort since we did so for the folder flow. The initial implementation read directly from Quick Access so there was no need to sort again. |
src/Files.App/ViewModels/UserControls/Widgets/RecentFilesWidgetViewModel.cs
Show resolved
Hide resolved
Do you know IApplicationDocumentLists? This is specifically enumeration interface for frequent and recent items. Also IApplicationDestinations is provided for deleting an item and clearing all. I have to try. |
I don't, but the API seems promising. From an glance, it looks like a bunch of |
@jiejasonliu Thank you for the review. Speaking of IApplicationDocumentLists, I confirmed this was a completely different stuff; File Explorer only adds recent 'folders'. Presumably, we still gotta to use a dedicated shell folder, as we have. |
This is ready for review again, tested everything on my end. One thing I leave as the future work is to use IStorable for recent items, determining if it's IFolder or IFile and invoke one appropriate verb or two. This is also the case for any other UI item classes that represent a storage object. |
Can you rebase with the latest from |
8ea304b
to
2754111
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like everything is working properly. @jiejasonliu do you have anything else to go over before I merge the PR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm! thanks for taking the time to not only refactor but to explore alternative ways to achieve similar behaviors.
Resolved / Related Issues
Note
There's been a known issue with enumerating recent items, where the service enumerates pinned files as well as recent items. Because of this, if there's any, Clear All doesn't work properly. While this PR didn't introduce this issue we probably want to look into later.
Steps used to test these changes