Skip to content

Add unified search options#30311

Merged
juliusknorr merged 2 commits intomasterfrom
enh/search-load
Apr 20, 2022
Merged

Add unified search options#30311
juliusknorr merged 2 commits intomasterfrom
enh/search-load

Conversation

@juliusknorr
Copy link
Member

@juliusknorr juliusknorr commented Dec 17, 2021 •
edited
Loading

  • Change debounce timeout to 500ms to trigger search less often when typing slowly
  • Allow to set custom value for minimum character limit
  • Allow to set custom value for number of results
  • Add option to disable type ahead functionality and only trigger search on enter/click on the submit button
Peek.2021-12-17.10-15.mp4 
occ config:app:set core unified-search.live-search --value=no
occ config:app:set core unified-search.min-search-length --value=4

@juliusknorr juliusknorr requested review from a team, Pytal, artonge and skjnldsv and removed request for a team December 17, 2021 09:16
@juliusknorr juliusknorr added 3. to review Waiting for reviews enhancement labels Dec 17, 2021
@juliusknorr juliusknorr added this to the Nextcloud 24 milestone Dec 17, 2021
@solracsf solracsf mentioned this pull request Dec 17, 2021
Closed
artonge
artonge reviewed Dec 17, 2021
View reviewed changes
Copy link
Contributor

@artonge artonge left a comment

Choose a reason for hiding this comment

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

Would rename typeAhead to something more explicit, like enableLiveSearch. But else, looks fine :).

PVince81
PVince81 approved these changes Dec 17, 2021
View reviewed changes
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.

👍 code looks fine

I agree with @artonge that "type-ahead" sounds more confusing than "live search"

@juliusknorr
Copy link
Member Author

juliusknorr commented Dec 17, 2021

Sure, I'll adjust to live search, thanks for the suggestion.

CarlSchwan
CarlSchwan approved these changes Dec 17, 2021
View reviewed changes
Copy link
Member

@CarlSchwan CarlSchwan left a comment

Choose a reason for hiding this comment

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

Code looks good!

skjnldsv
skjnldsv requested changes Dec 21, 2021
View reviewed changes
Copy link
Member

@skjnldsv skjnldsv left a comment

Choose a reason for hiding this comment

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

minSearchLength is not used? What is it being used for?

jancborchardt
jancborchardt approved these changes Dec 28, 2021
View reviewed changes
Copy link
Member

@jancborchardt jancborchardt left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Looks good design-wise when the settings are customized. :) Also ok on upping the debounce timeout.

@juliusknorr
Copy link
Member Author

juliusknorr commented Feb 7, 2022

minSearchLength is not used? What is it being used for?

It is used in the code already for letting the search only start after a certain amount of characters, just that it was made an option now to configure.

@juliusknorr juliusknorr requested a review from skjnldsv February 7, 2022 10:01
@juliusknorr
Copy link
Member Author

juliusknorr commented Feb 7, 2022

Rebased to latest master

@skjnldsv skjnldsv mentioned this pull request Mar 24, 2022
Merged
@blizzz blizzz mentioned this pull request Mar 31, 2022
Merged
@blizzz blizzz mentioned this pull request Apr 7, 2022
Merged
@szaimen
Copy link
Contributor

szaimen commented Apr 7, 2022

@juliushaertl what is missing here? :)

@juliusknorr
Copy link
Member Author

juliusknorr commented Apr 13, 2022

Just another rebase, pushed and waiting for CI.

@blizzz blizzz mentioned this pull request Apr 13, 2022
Merged
@szaimen
Copy link
Contributor

szaimen commented Apr 13, 2022

acceptance-access-levels is failing. Not sure if related though

@juliusknorr
Copy link
Member Author

juliusknorr commented Apr 19, 2022

Doesn't seem so, but restarted that drone run.

@szaimen
Copy link
Contributor

szaimen commented Apr 19, 2022

Seems to pass now..

@juliusknorr juliusknorr merged commit 8e53a04 into master Apr 20, 2022
@juliusknorr juliusknorr deleted the enh/search-load branch April 20, 2022 14:36
@juliusknorr
Copy link
Member Author

juliusknorr commented Apr 20, 2022

Let's get that in then :)

@szaimen szaimen modified the milestones: Nextcloud 24, Nextcloud 25 Apr 20, 2022
@PVince81
Copy link
Member

PVince81 commented Nov 18, 2022

@juliushaertl maybe we should backport this also ?

at least for the increased debounce

@juliusknorr
Copy link
Member Author

juliusknorr commented Nov 21, 2022

/backport to stable24

@juliusknorr
Copy link
Member Author

juliusknorr commented Nov 21, 2022

/backport to stable23

@backportbot-nextcloud
Copy link

backportbot-nextcloud bot commented Nov 21, 2022

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

@backportbot-nextcloud
Copy link

backportbot-nextcloud bot commented Nov 21, 2022

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

@juliusknorr
Copy link
Member Author

juliusknorr commented Nov 21, 2022

Will do them manually

@juliusknorr juliusknorr mentioned this pull request Nov 25, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@artonge artonge artonge left review comments

@PVince81 PVince81 PVince81 approved these changes

@jancborchardt jancborchardt jancborchardt approved these changes

@CarlSchwan CarlSchwan CarlSchwan approved these changes

@Pytal Pytal Awaiting requested review from Pytal

@skjnldsv skjnldsv Awaiting requested review from skjnldsv

Assignees

No one assigned

Labels

3. to review Waiting for reviews enhancement

Projects

None yet

Milestone

Nextcloud 25

Development

Successfully merging this pull request may close these issues.

7 participants

@juliusknorr @szaimen @PVince81 @jancborchardt @artonge @skjnldsv @CarlSchwan