Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

Support star, fix riot-web issue #2938 #1716

Closed
wants to merge 2 commits into from

Conversation

gotmyname2018
Copy link

This is part of work to fix riot-web issue 2938.
The basic idea is reuse the search function, but enhance it with search expression support.
The synapse side work Support search expression explains the search expression syntax.
A "Search starred" button was added to room header, once clicked, all starred messages will show as search results.
A "Cancel" button will be shown at AuxPanel to be used to close the search result panel.

@t3chguy
Copy link
Member

t3chguy commented Jan 25, 2018

This would surely require a spec proposal as synapse is not the only homeserver

@gotmyname2018
Copy link
Author

Whether raise a issue (improvement type) in matrix-doc, and a pull request with a updated matrix-doc/specification/modules/search.rst is enough?

@t3chguy
Copy link
Member

t3chguy commented Jan 25, 2018

I'd suggest first getting feedback on this approach, as in whether using /search for this is wanted by the wider community/core devs. Knowing your reasoning for this approach could shed some light on it

@gotmyname2018
Copy link
Author

Ya, so let me explains my reasoning :)
I first considered using specific api end point to fetch all starred messages, but after carefully study the current implementation (both riot-web, matrix-react-sdk and synapse), I find reuse /search gives many advantages:

  • You can combine plain key word and :starred to precisely look for specific starred messages, and in the future with even more search criteria.
  • The current rich functions of search result panel (such as pagination) could be bring to starred list with minimal cost
  • And this approach could be easily extended for future features (other tags: for e.g. a read it latter list)
  • And finally, it's a backward compatible change, the original searching semantic does not break.

@jryans
Copy link
Collaborator

jryans commented Jul 31, 2019

Thanks for making this contribution a while back. Since the code base has changed since this was opened, it no longer applies cleanly, and I don't think there's a need to keep it open in this state, as we can always find the code here again if needed. If you are still interested in pursuing this feature, please discuss with us in #riot-dev:matrix.org to find a good approach forward.

@jryans jryans closed this Jul 31, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants