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

feat: In app search #41609

Merged
merged 7 commits into from
Jun 27, 2024
Merged

feat: In app search #41609

merged 7 commits into from
Jun 27, 2024

Conversation

marcoambrosini
Copy link
Member

@marcoambrosini marcoambrosini commented Nov 20, 2023

Summary

This is an attempt to reconcile app search and global search into a single component and user flow.

Advantages of this approach:

  • One search box in the UI is less confusing than 2;
  • The search-box is on top of the content, no overlapping between search box container and search results
Screencast.from.2023-11-20.17-13-37.webm

On mobile

Collapsed Expanded
Screenshot from 2023-11-20 17-56-55 Screenshot from 2023-11-20 17-56-50

TODO

  • Limit this to apps that accept filtering
  • Use proper app name for placeholder
  • Expand transition for search box

@szaimen
Copy link
Contributor

szaimen commented Dec 2, 2023

I really like this idea! What is needed to finish this?

@marcoambrosini
Copy link
Member Author

@szaimen we'll have to address proper in app search strategy this for the next major. @jancborchardt had usability concerns about this solution. I think it's a pretty solid starting point compared to other alternatives, but of course I'm happy to iterate over this or explore other solutions and see how they compare.

@szaimen
Copy link
Contributor

szaimen commented Dec 11, 2023

@jancborchardt had usability concerns about this solution.

What kind of usability concerns?

I think it's a pretty solid starting point compared to other alternatives

Indeed, I think it is much better in terms of usability than the current situation.

@marcoambrosini
Copy link
Member Author

Closing as there's no need for a last minute solution anymore, and it seems that we all favor proper in app search that's tailored to the respective apps and thus slightly different in each case.

@skjnldsv skjnldsv deleted the enh/in-app-search branch March 14, 2024 07:52
@jancborchardt jancborchardt restored the enh/in-app-search branch April 4, 2024 16:35
@jancborchardt
Copy link
Member

@marcoambrosini I still think this could be a good combination of in-app and global search. It would also move us closer to the usual "big search field in the top center" that people know from Google and Microsoft et al. This was also a big point why you favored this approach, no @marcoambrosini?

Also cc @AndyScherzinger since we recently talked about search.

@jancborchardt jancborchardt reopened this Apr 4, 2024
@jancborchardt jancborchardt removed this from the Nextcloud 29 milestone Apr 4, 2024
@szaimen
Copy link
Contributor

szaimen commented Apr 4, 2024

@marcoambrosini I still think this could be a good combination of in-app and global search. It would also move us closer to the usual "big search field in the top center" that people know from Google and Microsoft et al. This was also a big point why you favored this approach, no @marcoambrosini?

I agree 👍

@susnux susnux added this to the Nextcloud 30 milestone Apr 18, 2024
@szaimen
Copy link
Contributor

szaimen commented May 17, 2024

I just tested this branch locally and it doesnt seem to work... @marcoambrosini would you mind updating it? :)

@susnux
Copy link
Contributor

susnux commented Jun 25, 2024

@szaimen there were some "heavy" adjustments needed to bring this API wise to a consistent state (not 100% related to this feature but we basically needed to split out the event handling to the main view and only handle the unified global search in the modal).

I tested it locally and it seems to work as expected, some styles are broken due to the clickable area refactoring, but will be fixed soon I think with new nextcloud-vue version.

Bildschirmaufnahme_20240625_211909.webm

@szaimen

This comment was marked as resolved.

@marcoambrosini
Copy link
Member Author

Could the input be always shown and centered if there's enough space?
By enough space I mean showing a few apps and then clipping the list

@marcoambrosini
Copy link
Member Author

very important too: Does this then also work in apps management, account management, and other apps with existing in-app search like Talk, Contacts, Mail, etc?

We would need to remove local search from those apps if we go with this imo

@susnux
Copy link
Contributor

susnux commented Jun 26, 2024

very important too: Does this then also work in apps management, account management, and other apps with existing in-app search like Talk, Contacts, Mail, etc?

Yes it works for all apps that implement the already used "filter in current view" (yet we need proper API ...)

@susnux susnux changed the title [Proof of concept] In app search feat: In app search Jun 26, 2024
@susnux
Copy link
Contributor

susnux commented Jun 26, 2024

sounds good with cloud-search!

Bildschirmfoto_20240626_204122

marcoambrosini and others added 7 commits June 27, 2024 12:13
Co-authored-by: Ferdinand Thiessen <opensource@fthiessen.de>
Signed-off-by: Marco Ambrosini <marcoambrosini@icloud.com>
Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
Co-authored-by: Simon L. <szaimen@e.mail.de>
Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
…h:reset` event

Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
@szaimen szaimen merged commit a184161 into master Jun 27, 2024
110 checks passed
@szaimen szaimen deleted the enh/in-app-search branch June 27, 2024 11:34
Copy link
Member

@jancborchardt jancborchardt left a comment

Choose a reason for hiding this comment

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

@susnux @szaimen just before you put more time into this – we talked about this again among @AndyScherzinger @karlitschek and me, and in addition to the considerations in the previous comment we think that in the long term it would be better to have 2 searches:

  • One present search field inside the app, like it is in Talk, Contacts, Mail, Collectives, etc.
  • One search icon on the top right for the Unified search

And to do that, we would "simply" need to add in-app searches in:

This would also resolve the confusion with the bit strange interaction pattern of having an input field with a button that does something else, and with internal search not existing in specific apps and due to that the flow being different.

This is very comparable to how for example macOS or GNOME does it with every app having in-app search, and there being a global search (like Spotlight) in case you don’t find it.

@szaimen @susnux sorry for the back and forth – it just turns out it is way more complex than thought. :\

@szaimen
Copy link
Contributor

szaimen commented Jul 26, 2024

Hi @susnux this is not working on daily.ltd2.nextcloud.com anymore. Was it removed?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews enhancement
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

7 participants