typeahead: Properly handle custom triggers.

[aero31aero]

Fixes #14246.

This fixes an issue in our stream-topic typeahead code where sometimes typing #den> wouldn't complete the stream mention.

I've added the second commit here to run tests properly.

[showell]

I wonder if this would actually be more maintainable if we just used undefined for this.custom_selection_state rather than having to give falsy values (of three different types!) for the three fields.

I would also make the commenting here slightly more verbose, as it's hard for somebody jumping in this code to know what's going on.

The variable name custom_selection_state is possibly confusing me here, because when I hear "selection" I think copy/paste instead of choosing, and I think this is more about choosing, correct?

[showell]

I guess "selection" is already well explained in the big block comment (which I didn't see in the diff), so disregard that part of my prior comment.

[aero31aero]

I was on the fence for using selection here, but the typeahead function is called this.select() so it made sense to follow the convention and say trigger_selection. Anyway, updated the default to be an empty object. This avoids having to write code like if (this.x and this.x.y) and also avoids having to specify any defaults.

I would also make the commenting here slightly more verbose, as it's hard for somebody jumping in this code to know what's going on.

This is why I added a reference to the issue where I've written a larger write-up. Since the write-up was large enough, I figured it made sense to have it in a rendered markdown format instead of an in-file comment. (#14246).

To make sure people stumble on it, I added the comment at the top of the file as well now.

[timabbott]

This code is kinda hard to follow. Are we confident that this.custom_selection_state is always set during a keyup call? And is the keyCode in custom_selection_state used for anything?

[aero31aero]

Yes, a keyup event implies that we have already had a keypress event.

[zulipbot]

Heads up @aero31aero, we just merged some commits that conflict with the changes your made in this pull request! You can review this repository's recent commits to see where the conflicts occur. Please rebase your feature branch against the upstream/main branch and resolve your pull request's merge conflicts accordingly.