Skip to content
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

Implement local filtering in file list #26841

Merged
merged 3 commits into from
Oct 26, 2021
Merged

Conversation

juliusknorr
Copy link
Member

@juliusknorr juliusknorr commented Apr 30, 2021

Bring back local file list filtering which got removed with the implementation of unified search. It additionally introduces a bundled entrypoint for the files app which now unifies search handling, templates and the files app settings in the left navigation sidebar.

@nickvergessen
Copy link
Member

nickvergessen commented Apr 30, 2021

There is no indication once the unified search is closed that the file list is filtered

Bildschirmfoto von 2021-04-30 11-32-04

So in theory there is, but it might not be as visible as it used to be

@nickvergessen
Copy link
Member

Fixed the problem with the broken "No entries in this folder" message

Before After
Bildschirmfoto von 2021-04-30 11-49-15 Bildschirmfoto von 2021-04-30 11-51-10

@nickvergessen
Copy link
Member

Switching between file lists might need to be handled with special care so that the search query gets updated on all file lists

So yeah, it doesn't work fully, but it is better than on 19. On 19 the search box is emptied and visible but the filter is still applied, giving you even more headache :P

From my POV this is already a huge step forward and should be merged + backported.
Fixing the filter to work completely across pages in all cases can be done later

@PVince81
Copy link
Member

Fixes #23432

@juliusknorr
Copy link
Member Author

I'll be leaving the follow up points for later as I agree with @nickvergessen that this is already a huge step forward and the old implementation had similar flaws when switching the file list:

  • There is no indication once the unified search is closed that the file list is filtered
  • Switching between file lists might need to be handled with special care so that the search query gets updated on all file lists

@juliusknorr
Copy link
Member Author

Diff in case it might be useful to make sure that newly initialized filelists also have the filter, but there are some sideeffects when it comes to the loading indicator:

 import { subscribe } from '@nextcloud/event-bus'
 
+let searchQuery = ''
+subscribe('nextcloud:unified-search.search', ({ query }) => {
+       searchQuery = query ?? ''
+});
+
 (function() {
 
        const FilesPlugin = {
                attach(fileList) {
+                       // Initialize the file list with the current query in case there was a search before switching to the list
+                       fileList.setFilter(searchQuery)
+
                        subscribe('nextcloud:unified-search.search', ({ query }) => {
                                fileList.setFilter(query)
                        })

@juliusknorr juliusknorr marked this pull request as ready for review October 25, 2021 14:55
@juliusknorr juliusknorr changed the title WIP: Implement local filtering in file list Implement local filtering in file list Oct 25, 2021
@juliusknorr juliusknorr requested review from skjnldsv, a team, artonge and Pytal and removed request for a team October 25, 2021 14:56
@juliusknorr juliusknorr added 3. to review Waiting for reviews bug regression and removed 2. developing Work in progress labels Oct 25, 2021
@juliusknorr juliusknorr added this to the Nextcloud 23 milestone Oct 25, 2021
@juliusknorr
Copy link
Member Author

/backport to stable22

@juliusknorr
Copy link
Member Author

/backport to stable21

@juliusknorr
Copy link
Member Author

/backport to stable20

@Pytal
Copy link
Member

Pytal commented Oct 26, 2021

👌

Copy link
Member

@PVince81 PVince81 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested, works 👍

It's only strange that the dropdown also shows the same results after a while, but no biggie

@PVince81 PVince81 merged commit f9629c0 into master Oct 26, 2021
@PVince81 PVince81 deleted the bugfix/files-search-filter branch October 26, 2021 06:23
@backportbot-nextcloud
Copy link

The backport to stable22 failed. Please do this backport manually.

@backportbot-nextcloud
Copy link

The backport to stable21 failed. Please do this backport manually.

@backportbot-nextcloud
Copy link

The backport to stable20 failed. Please do this backport manually.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants