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

Username autocompletion #82

Open
wants to merge 13 commits into
base: master
Choose a base branch
from

Conversation

chucker
Copy link

@chucker chucker commented Jun 3, 2020

Suggest users when typing @ followed by part of a user name or display name.

Multiple aspects required:

  • we need a UI like Gitter's own, or like in https://github.com/Blazored/Typeahead/ (but we can't outright use Typeahead, I believe, because it seems to always require a drop-down rather than being invoked inline from the message input box). Stuff like arrow up/down to select someone, etc.

  • we need JS interop to get the cursor position (I think)

  • we need the API-side search. Unlike Gitter's own client, this PR takes advantage of Cache room users? #75 so results are more comprehensive.

@chucker
Copy link
Author

chucker commented Jun 4, 2020

A very early version of the UI is now here. The positioning is, too, but doesn't seem to actually work yet.

@chucker chucker marked this pull request as ready for review June 5, 2020 10:34
@chucker
Copy link
Author

chucker commented Jun 5, 2020

I've removed draft status because this is way beyond proof of concept now.

Remaining issues before this should be merged:

  • add a way to select a suggested user and autocomplete the input box that selection. Probably up/down arrow keys, then return. Maybe also clicking/tapping?

Remaining issues that I don't think should block a merge:

  • the cache doesn't discriminate by room. If you're in multiple rooms, it'll suggest users that aren't actually here. This is technically a design flaw, but I'm not sure it matters much in practice — you probably know whom you want to @. I can't think of a way to rectify this short of significantly complicating the data structure.
  • the UI isn't pretty yet.
  • horizontal alignment doesn't work. I feel like I'm missing a stupidly obvious way here; I can only think of a measureString approach (measure the text up until before the @, then left-align the popup there). That would work, but seems overcomplicated?

@SQL-MisterMagoo
Copy link
Collaborator

For the user list, I would personally be happy to let it pop up where you have it - the horizontal doesn't matter too much, but if you want to try and position it, I found this that seems to work https://medium.com/@jh3y/how-to-where-s-the-caret-getting-the-xy-position-of-the-caret-a24ba372990a

@chucker
Copy link
Author

chucker commented Jun 5, 2020

if you want to try and position it, I found this that seems to work https://medium.com/@jh3y/how-to-where-s-the-caret-getting-the-xy-position-of-the-caret-a24ba372990a

Yeah, the offscreen div approach is one I evaluated. I'll consider it.

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

Successfully merging this pull request may close these issues.

2 participants