Page MenuHomePhabricator

[native] Show chats in mention typeahead tooltip
AcceptedPublic

Authored by patryk on Aug 22 2023, 5:26 AM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Sep 17, 3:34 AM
Unknown Object (File)
Sun, Sep 17, 3:06 AM
Unknown Object (File)
Fri, Sep 15, 3:56 PM
Unknown Object (File)
Fri, Sep 15, 9:24 AM
Unknown Object (File)
Fri, Sep 15, 1:22 AM
Unknown Object (File)
Fri, Sep 15, 12:03 AM
Unknown Object (File)
Thu, Sep 14, 8:39 PM
Unknown Object (File)
Thu, Sep 7, 9:22 AM
Subscribers

Details

Reviewers
tomek
inka
rohan
Summary

Solution for ENG-4550.

This diff enables showing chats in typeahead tooltip.

Depends on D8909.

Test Plan

Test if tooltip:

  1. contains chat names
  2. doesn't contain chat in which we currently are
  3. changes its content depending on text prefix

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

patryk held this revision as a draft.
patryk edited the test plan for this revision. (Show Details)
patryk added reviewers: tomek, inka, rohan.
native/utils/typeahead-utils.js
9 ↗(On Diff #30216)

This regex has one drawback: when we type '@' in chat input bar, getMentionTypeahead(User/Chat)Suggestions will always execute. But there is no way to avoid this -> previously when we me mentioned users only we could stop SearchIndex lookup when user types space after mention -> right now when threads can have spaces it's impossible. validChatNameRegexString has a limitation to 191 chars, so when user types a very long message after mention, eventually it will stop executing mention utilities.

patryk published this revision for review.Aug 22 2023, 8:39 AM
const suggestions = [
  ...suggestedUsers.map(user => ({ type: 'user', user })),
  ...suggestedChats.map(chat => ({ type: 'chat', chat })),
];

This map can be omitted by changing return type of getMentionTypeahead(User/Chat)Suggestions function.

Rebase and match refactored component style

lib/shared/mention-utils.js
115 ↗(On Diff #30293)

Can you make a Set out of this to make the search in line 118 constant instead of linear?

Refactor getMentionTypeaheadChatSuggestions

native/utils/typeahead-utils.js
19 ↗(On Diff #30686)

Same question: isn't oldValidUsernameRegexString a subset of validChatNameRegexString?

lib/shared/mention-utils.js
133 ↗(On Diff #30686)

Does the ordering of the result matter? Are the results sorted?

native/chat/chat-input-bar.react.js
578 ↗(On Diff #30686)

Is it a good idea that we them in this order?

lib/shared/mention-utils.js
133 ↗(On Diff #30686)

Results are not sorted, but we might want to sort them like we do for user candidates.

native/chat/chat-input-bar.react.js
578 ↗(On Diff #30686)

Last sentence of this comment states that we need to show user mentions before chat mentions. But what we could do is to sort suggestions array and place user mentions before chat mentions (if mention text is the same).

native/utils/typeahead-utils.js
19 ↗(On Diff #30686)

Yes it is, will remove it

Remove oldValidUsernameRegexString from mention regex

tomek added inline comments.
lib/shared/mention-utils.js
115

Maybe chatNamePrefix?

This revision is now accepted and ready to land.Tue, Sep 26, 7:50 AM