Skip to content

Conversation

@nickvergessen
Copy link
Member

@nickvergessen nickvergessen commented Aug 17, 2020

Bildschirmfoto von 2020-08-20 20-38-03

  • Simple search for messages (no files/shares shown)
  • Avatar of poster for users, talk icon for guests (for now)
  • No filter to current conversation possible (should have a short cut like in:<conversation> ?)
  • Trim message in case longer than X chars, limiting to "relevant" part + pre + post?
  • Filter placeholders mention-*- and don't search for it?

Follow ups

  • Avatar for guests
  • Scroll to the relevant message on clicking?

@nickvergessen nickvergessen added 2. developing feature: chat 💬 Chat and system messages feature: integration 📦 Integration with 3rd party (chat) service labels Aug 17, 2020
@nickvergessen nickvergessen added this to the 💚 Next Major (20) milestone Aug 17, 2020
@nickvergessen
Copy link
Member Author

@nextcloud/designers-talk

Some questions:

  • Due to technical reasons mentions are stored as {mention-user-NUMBER}, searching for mention, user, and numbers will create dozens of "false positives". Adding a regex around it to lookahead/lookbehind is not feasable performance wise, so I would not search in talk, when that is what you type. Is that okay?
  • Should searching for talk searchsearch for "talk search" or "talk" + "search"? Currently the first thing is done and it's the same in all other apps I guess?
  • What is a feasible length to trim messages at? We can't throw a "up to 64k chars" message in the search…
  • Time when the message was posted is the order by at the moment, should we still also display it in the subline? Or is that too much details?

@jancborchardt
Copy link
Member

I would not search in talk, when that is what you type. Is that okay?

So you mean when searching for the literal words "mention" and "user" and any number?
I’d say that’s alright as the first thing, but we should definitely be able to search for these things. Especially numbers are quite relevant, e.g. when talking about account numbers, issue numbers, package tracking, or whatnot is done in organizations.

Should searching for talk searchsearch for "talk search" or "talk" + "search"? Currently the first thing is done and it's the same in all other apps I guess?

@skjnldsv how do the other apps do it, or rather the other solutions you checked out for this? This should not be handled by the apps but by unified search centrally.

What is a feasible length to trim messages at? We can't throw a "up to 64k chars" message in the search…

What is the max length of messages, 64k chars? And does trimming mean that we will not search past that part of the message (which is not so good) or does it just mean that we will not be able to show the relevant part in the subline of the search result, but still show it (that’s fine).

Time when the message was posted is the order by at the moment, should we still also display it in the subline? Or is that too much details?

Yes, that’s good as the order – with most recent at the top. Not necessary to display that in the subline, indeed too much details. :)

@skjnldsv
Copy link
Member

@skjnldsv how do the other apps do it, or rather the other solutions you checked out for this? This should not be handled by the apps but by unified search centrally.

honestly, no idea, each app can handle its own data.
This is a whole dedicated job to properly manage search results and know algorithms that have proper relevancy :)

@jancborchardt
Copy link
Member

Well then I’d say that a search for talk search should be handled like "talk" + "search" as it’s fuzzier, while restricting it to "talk search" would get too few results or none. E.g. often you might search for topic person, disjointed keywords you remember, not things which actually were directly said after each other.

@rullzer
Copy link
Member

rullzer commented Aug 18, 2020

Well then I’d say that a search for talk search should be handled like "talk" + "search" as it’s fuzzier, while restricting it to "talk search" would get too few results or none. E.g. often you might search for topic person, disjointed keywords you remember, not things which actually were directly said after each other.

This really is not a topic to discuss here.

@rullzer
Copy link
Member

rullzer commented Aug 18, 2020

What is the max length of messages, 64k chars? And does trimming mean that we will not search past that part of the message (which is not so good) or does it just mean that we will not be able to show the relevant part in the subline of the search result, but still show it (that’s fine).

If you write messages longer than 64k chars then it is your own fault if things break down

@nickvergessen
Copy link
Member Author

Well well well, that escalated quickly.

The char limit is not about search, but how much we should show in the result.

does it just mean that we will not be able to show the relevant part in the subline of the search result, but still show it (that’s fine).

Also the message is not the subline at the moment, but the title, as that is where your search match is in. subline is for orientation details (copied from comments)

@nickvergessen

This comment has been minimized.

@nickvergessen
Copy link
Member Author

I switched title and subline now, and trim the message, if it is longer than 40 chars and the search term is not in the first 30:

Bildschirmfoto von 2020-08-18 20-15-15

@rullzer
Copy link
Member

rullzer commented Aug 19, 2020

Looks really nice!
LGTM :D

@nickvergessen nickvergessen added feature: search 🔎 and removed feature: integration 📦 Integration with 3rd party (chat) service labels Aug 19, 2020
@nickvergessen
Copy link
Member Author

Missing to jump to the respective message but that's also the case for quotes, so let's do that as a follow up.

@nickvergessen nickvergessen force-pushed the feature/noid/search-for-messages branch from e9de471 to a77bef4 Compare August 20, 2020 18:32
@nickvergessen
Copy link
Member Author

Still requires nextcloud/server#22111 in server

@nickvergessen nickvergessen marked this pull request as ready for review August 20, 2020 18:35
Signed-off-by: Joas Schilling <[email protected]>
Copy link
Member

@marcoambrosini marcoambrosini left a comment

Choose a reason for hiding this comment

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

For me clicking the search result doesn't route to the conversation

@nickvergessen
Copy link
Member Author

I pushed right before your comment

Copy link
Member

@marcoambrosini marcoambrosini left a comment

Choose a reason for hiding this comment

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

As said let's get this in and do the fetching and scrolling in a followup

@nickvergessen nickvergessen merged commit e80847e into master Aug 21, 2020
@nickvergessen nickvergessen deleted the feature/noid/search-for-messages branch August 21, 2020 12:48
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.

6 participants