Page MenuHomePhabricator
Differential D8944

[web] Modify mention typeahead regex
AcceptedPublic
Actions

Authored by patryk on Aug 25 2023, 3:11 AM.
Tags
None
Referenced Files
Unknown Object (File)
Sat, Sep 16, 2:43 AM
Unknown Object (File)
Fri, Sep 15, 5:16 PM
Unknown Object (File)
Sun, Sep 3, 7:54 AM
Unknown Object (File)
Sat, Aug 26, 6:08 PM
Subscribers
ashoat
tomek

Details

Reviewers
tomek
inka
rohan
Summary

This diff modifies mention typeahead regex on web to support chat mentioning.

Depends on D8943.

Test Plan

Tested later in the stack.

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

patryk created this revision.Aug 25 2023, 3:11 AM
patryk held this revision as a draft.
Herald added subscribers: tomek, ashoat. · View Herald TranscriptAug 25 2023, 3:11 AM
patryk edited the summary of this revision. (Show Details)Aug 25 2023, 3:16 AM
patryk added reviewers: tomek, inka, rohan.
patryk added a parent revision: D8943: [web] Refactor TypeaheadTooltip component.
patryk added a child revision: D8945: [web] Show chats in mention typeahead tooltip.
patryk edited the summary of this revision. (Show Details)Aug 25 2023, 3:23 AM
patryk edited the test plan for this revision. (Show Details)
Harbormaster completed remote builds in B22080: Diff 30308.Aug 25 2023, 3:29 AM
patryk published this revision for review.Aug 25 2023, 4:01 AM
patryk added inline comments.
web/utils/typeahead-utils.js
22 ↗(On Diff #30308)

Potential drawbacks of this solution are described here.

rohan added a comment.Aug 25 2023, 6:19 AM

I think it'll be useful to add some unit tests for this regex, alongside the regex in D8834

inka added inline comments.Wed, Sep 6, 1:45 AM
web/utils/typeahead-utils.js
22 ↗(On Diff #30308)

Is there any point is using an alternative here? Isn't oldValidUsernameRegexString a subset of validChatNameRegexString?

patryk updated this revision to Diff 31255.Tue, Sep 19, 3:48 AM

Remove oldValidUsernameRegexString.

Harbormaster completed remote builds in B22719: Diff 31255.Tue, Sep 19, 4:05 AM
ashoat added a comment.Tue, Sep 19, 7:27 AM
In D8944#263772, @rohan wrote:

I think it'll be useful to add some unit tests for this regex, alongside the regex in D8834

This comment doesn't appear to have been addressed

patryk added a comment.Fri, Sep 22, 7:29 AM

D9057

inka accepted this revision.Fri, Sep 22, 7:32 AM
This revision is now accepted and ready to land.Fri, Sep 22, 7:32 AM

Revision Contents
Changeset List

PathSize
web/
chat/
2 lines
utils/
4 lines

Diff 31255

View Options

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

Loading...
View Options

web/utils/typeahead-utils.js

Loading...