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

Mentions polish #4582

Merged
merged 12 commits into from
Jan 25, 2019
Merged

Mentions polish #4582

merged 12 commits into from
Jan 25, 2019

Conversation

brianlovin
Copy link
Contributor

Status

  • WIP
  • Ready for review
  • Needs testing

Deploy after merge (delete what needn't be deployed)

  • hyperion (frontend)

When I put this on alpha I noticed some bugs:

  • It was possible to show tons of suggestions, where the suggestions list would go off screen
  • The participants list wouldn't load for an initial thread in the inbox because of SSR and the fact that we were populating participants in cDU instead of cDM
  • If the user typed @f and then hit backspace to have @ we were clearing all participants instead of restoring the original list of message senders + author
  • If the user switched threads from one with many message to one with no messages, we weren't resetting the participants list to only contain the thread author
  • The styling was confusing because we were rending usernames but not full names; this was confusing when trying to mention someone whose name and username don't match. I fixed this by just rendering both

@brianlovin
Copy link
Contributor Author

brianlovin commented Jan 25, 2019

9cf4048 fixes a problem where if a user's name and username are very different, the search results would be wonky because we only sorted by username. E.g. if a user's name is "Phil" but their username is "blah", if you searched for @phil no results would show up, since username.indexOf('phil') is -1. I've included names in the sorting function, ensuring that they are lowercased. Additionally, I always sort results if the search query is at the 0 index of either the person's username or name, e.g. a search for Phil will always prioritize results whose name or username start with Phil, rather than simply containing the string. As a result "Phil Smith" will always rank higher than "Theophilus Jones".

@brianlovin
Copy link
Contributor Author

38989e3 fixes a bug where if somehow a server search doesn't return results that definitely exist in the local participants list, we at least show the local results as suggestions. This solves for edge cases where Algolia may be broken, Vulcan may be out of sync, or the API slows/fails a search query - at least in these situations local results will appear for autocompletion.

@brianlovin
Copy link
Contributor Author

@mxstbr given thorough testing locally here, and ability to iterate on the search results as we find problems, I'm going to admin ship this and complete the prod cut for 2.6.2 :)

@brianlovin brianlovin merged commit e5941bd into alpha Jan 25, 2019
@brianlovin brianlovin deleted the mentions-polish branch January 25, 2019 23:59
@phil
Copy link

phil commented Feb 2, 2019

The irony of fixing mentions is strong with PR

@mxstbr
Copy link
Contributor

mxstbr commented Feb 2, 2019

How do you mean @phil?

@brianlovin
Copy link
Contributor Author

brianlovin commented Feb 2, 2019 via email

@mxstbr
Copy link
Contributor

mxstbr commented Feb 2, 2019

Oh 🤣

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants