[web] Hide typeahead if multiple characters are selected
ClosedPublic
Actions

Authored by • przemek on Feb 3 2023, 5:06 AM.

Details

Reviewers

inka
michal
kamil

Commits

rCOMM28423a51b376: [web] Hide typeahead if multiple characters are selected

Summary

As discussed here: https://linear.app/comm/issue/ENG-2891/refactor-some-of-the-native-typeahead-creation-logic-and-make-web
and here: https://linear.app/comm/issue/ENG-2888/typeahead-decide-what-to-do-with-multi-character-selection-near-symbol
we needed a little refactor in web typeahead.
This diff resolves both.
It is possible to rewrite condtional event tighter, but we would need to wrap them in signle useMemo
which might hurt our performance. This way feel clear enough and we use utility function prepared earlier.

Test Plan

Tested if web typeahead works propery:

Diff Detail

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

• przemek created this revision.Feb 3 2023, 5:06 AM

Herald added subscribers: atul, tomek, ashoat. · View Herald TranscriptFeb 3 2023, 5:06 AM

• przemek retitled this revision from [web] Typeahead doesn't show if multiple charcaters are selected. to [web] Hide typeahead if multiple characters are selected.Feb 3 2023, 5:07 AM

• przemek edited the test plan for this revision. (Show Details)

• przemek attached a referenced file: F359975: Screen Recording 2023-02-03 at 14.00.10.mov. (Show Details)

Harbormaster completed remote builds in B16065: Diff 21939.Feb 3 2023, 5:20 AM

• przemek requested review of this revision.Feb 3 2023, 5:20 AM

Looks good to me (although the code comment now feels a little bit unnecessary).

inka accepted this revision.Feb 8 2023, 3:16 AM

inka added inline comments.

lib/shared/typeahead-utils.js
23–24 ↗	(On Diff #21939)	I'd change that a little bit, but it's up to you

This revision is now accepted and ready to land.Feb 8 2023, 3:16 AM

Deleted comment as it was redundant.

• przemek marked an inline comment as done.Feb 8 2023, 4:35 AM

• przemek added inline comments.

lib/shared/typeahead-utils.js
23–24 ↗	(On Diff #21939)	I decided to delete the comment entirely as the code here is self-explanatory

Harbormaster completed remote builds in B16278: Diff 22273.Feb 8 2023, 4:46 AM

Closed by commit rCOMM28423a51b376: [web] Hide typeahead if multiple characters are selected (authored by • przemek). · Explain WhyFeb 8 2023, 5:51 AM

This revision was automatically updated to reflect the committed changes.

• przemek marked an inline comment as done.

• przemek added a commit: rCOMM28423a51b376: [web] Hide typeahead if multiple characters are selected.

Revision Contents
Changeset List

Path

Size

web/

chat/

chat-input-bar.react.js

29 lines

Diff 22274

View Options

web/chat/chat-input-bar.react.js

[web] Hide typeahead if multiple characters are selectedClosedPublicActions