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

SearchBar: search by author #4156

Closed
wants to merge 21 commits into from

Conversation

Pestdoktor
Copy link

@Pestdoktor Pestdoktor commented Mar 1, 2020

Update 2020-09-16

Preview: https://element.fentker.eu

This PR replaces the SearchBar input field with a BasicMessageComposer, hooking up autocompleted user and room pills to search filters. Server-side search already implements these filters, seshat support may be found at matrix-org/seshat#71.

UX consideration

The ability to filter is not obvious to the user, especially after removing the room filter buttons

Possible Solutions:

  • Add new buttons ("in", "by") opening the autocompleter
  • Autofill the pill of the current room on opening the SearchBar
  • ... (Suggestions?)

Screenshot

image

Original

related to https://github.com/vector-im/riot-web/issues/4752

Todo

  • add styling
  • make filter removeable
  • local search support

Status

image

@turt2live turt2live changed the title [Draft] SearchBar: search by author [WIP] SearchBar: search by author Mar 2, 2020
@Pestdoktor
Copy link
Author

So this was much easier than I expected and is basically in a mergeable state already now AFAIC

Status

image

A follow-up would probably address accessibility, local search support and autocompleting members of all rooms instead of just the current one

@turt2live
Copy link
Member

This also needs product and design review to make sure it's something we actually want, and behaves how we want.

@turt2live turt2live requested review from a team March 2, 2020 16:01
@Pestdoktor Pestdoktor changed the title [WIP] SearchBar: search by author SearchBar: search by author Mar 4, 2020
@Pestdoktor
Copy link
Author

I just saw that local search isn't implemented yet, so I just added a commit preparing the noop for eventual by-author-support. At this point I'm going to lock my branch and commit any of the improvements I mentioned earlier to a new one so this one can be reviewed properly.

@turt2live turt2live removed the request for review from a team March 4, 2020 17:21
@turt2live
Copy link
Member

Taking this out of dev review so the design/product team can take a look at it

@nadonomy
Copy link
Contributor

nadonomy commented Mar 4, 2020

Is there somewhere we can review an ad hoc/development build of this?

@Pestdoktor
Copy link
Author

https://riot.fentker.eu/

@nadonomy
Copy link
Contributor

nadonomy commented Mar 5, 2020

https://riot.fentker.eu/

Thanks for providing this! Will review this soon. Relatedly, I've also opened up an issue here so we can better communicate the value of the ad hoc builds here: element-hq/element-web#12614.

@Pestdoktor
Copy link
Author

I'm looking to work on expanding this this week. Any feedback I should take into account?

Also what's the process to update the filter specs (https://github.com/matrix-org/matrix-doc/issues/969)? Can it just be implemented into Synapse as unspecified behaviour until there's a spec for it?

@nadonomy
Copy link
Contributor

I'm looking to work on expanding this this week. Any feedback I should take into account?

For sure, apologies reviewing this in detail is stuck behind some other work which is on the critical path but I'll figure out a way to unblock this.

@Pestdoktor Pestdoktor force-pushed the author-search branch 3 times, most recently from d535efe to b84802d Compare July 8, 2020 18:06
@Pestdoktor
Copy link
Author

I added a commit removing the search_term requirement, which is nice for testing but requires Pestdoktor/synapse@1584f7f and breaks spec-compliance so it should eventually be removed.

@Pestdoktor
Copy link
Author

I'm now using BasicMessageComposer instead of trying to replicate the autocomplete functionality.

I also updated again, hence the preview is now available at https://element.fentker.eu/ 😉

Next up would be to allow for specifying multiple senders (currently takes only first one) and also hook up the room filter the same way. After that I'd look into seshat to make it work with e2e search.

@Pestdoktor Pestdoktor force-pushed the author-search branch 6 times, most recently from e9b2a3d to 6cfdb1c Compare August 2, 2020 14:51
Signed-off-by: Jonas Fentker <jonas@fentker.eu>
Signed-off-by: Jonas Fentker <jonas@fentker.eu>
Signed-off-by: Jonas Fentker <jonas@fentker.eu>
Signed-off-by: Jonas Fentker <jonas@fentker.eu>
Signed-off-by: Jonas Fentker <jonas@fentker.eu>
Signed-off-by: Jonas Fentker <jonas@fentker.eu>
to avoid overflow issues with Autocomplete

Signed-off-by: Jonas Fentker <jonas@fentker.eu>
Signed-off-by: Jonas Fentker <jonas@fentker.eu>
Signed-off-by: Jonas Fentker <jonas@fentker.eu>
Signed-off-by: Jonas Fentker <jonas@fentker.eu>
Signed-off-by: Jonas Fentker <jonas@fentker.eu>
Signed-off-by: Jonas Fentker <jonas@fentker.eu>
Signed-off-by: Jonas Fentker <jonas@fentker.eu>
Signed-off-by: Jonas Fentker <jonas@fentker.eu>
Signed-off-by: Jonas Fentker <jonas@fentker.eu>
Signed-off-by: Jonas Fentker <jonas@fentker.eu>
Signed-off-by: Jonas Fentker <jonas@fentker.eu>
Signed-off-by: Jonas Fentker <jonas@fentker.eu>
Signed-off-by: Jonas Fentker <jonas@fentker.eu>
Signed-off-by: Jonas Fentker <jonas@fentker.eu>
@brunakov
Copy link

Great feature needed!

@kittykat
Copy link
Contributor

kittykat commented Apr 7, 2022

Product input: we would be open to adding the filtering if it fits with the designs for filtering in the new search experience. @Pestdoktor would you be interested in opening an updated PR for this issue?

As the codebase has changed a lot in the last couple of years, I'll close this one for now.

@kittykat kittykat closed this Apr 7, 2022
@niquewoodhouse niquewoodhouse removed their assignment Jul 15, 2022
@gaelledel gaelledel removed request for a team July 20, 2022 14:41
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.

6 participants