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

Add support for searching emoji and reactions #13643

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

innovate-invent
Copy link
Contributor

@innovate-invent innovate-invent commented Aug 5, 2024

First time contributor checklist

Contributor checklist

  • Virtual device Pixel_3a_API_34_extension_level_7_x86_64
  • My contribution is fully baked and ready to be merged as is
  • I ensure that all the open issues my contribution fixes are mentioned in the commit message of my first commit using the Fixes #1234 syntax

Description

Is it quite convenient to react to messages I want to refer back to with specific emoji (such as 📌).

This PR adds the Unicode emoji character class to the message indexing filter, including emoji in messages in the search index.
It also includes reaction emoji as a separate column in the search index.

I tested adding, changing, and removing reactions to a message and the interaction with the search results. I also tested deleting a message with a reaction.

I am not clear how Signal updates its database after a schema change. This CR works for new installs but fullyResetTables needs to be called in the event the app is being updated.

If this is accepted I intend to cut another PR that will allow multiple different reactions to a message from the same sender*.

@innovate-invent
Copy link
Contributor Author

innovate-invent commented Aug 5, 2024

I just realized I am missing LIMIT 1 on the insert and delete queries, fixing that and re-testing..

@innovate-invent innovate-invent marked this pull request as draft August 5, 2024 05:36
@greyson-signal
Copy link
Contributor

I don't think we want to add searching for reactions just yet, but adding the character classes for emoji looks very useful. I don't know if we want S*, but Sc and So seem like they would cover all of our needs! I'll look at taking that specific part and adding the database migration, and will give you co-authorship. Thanks!

@innovate-invent
Copy link
Contributor Author

Can you elaborate on why searching reactions shouldn't be added? It seems like a valuable feature.

@greyson-signal
Copy link
Contributor

I'd want to consider what the proper UX would be

@innovate-invent
Copy link
Contributor Author

What would motivate that consideration?

@innovate-invent innovate-invent marked this pull request as ready for review August 14, 2024 03:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants