Page MenuHomePhabricator
Differential D8943

[web] Refactor TypeaheadTooltip component
AcceptedPublic
Actions

Authored by patryk on Aug 25 2023, 3:11 AM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Sep 7, 10:32 AM
Unknown Object (File)
Sat, Sep 2, 4:43 AM
Unknown Object (File)
Tue, Aug 29, 8:54 PM
Subscribers
ashoat
tomek

Details

Reviewers
tomek
inka
rohan
Summary

This diff refactors TypeaheadTooltip component, like in D8897. New refactored component reuses new types introduced in that diff.

Depends on D8908.

Test Plan

Check if typeahead tooltip on web works properly.

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:14 AM
patryk added reviewers: tomek, inka, rohan.
patryk added a parent revision: D8908: [lib] Encode chat name in raw chat mention.
patryk added a child revision: D8944: [web] Modify mention typeahead regex.
patryk edited the summary of this revision. (Show Details)Aug 25 2023, 3:21 AM
patryk edited the test plan for this revision. (Show Details)
Harbormaster completed remote builds in B22079: Diff 30307.Aug 25 2023, 3:28 AM
patryk edited the summary of this revision. (Show Details)Aug 25 2023, 3:30 AM
patryk published this revision for review.Aug 25 2023, 4:01 AM
rohan requested changes to this revision.Tue, Aug 29, 10:31 AM

Just 1 question inline, requesting changes so it doesn't get lost in the stack

web/chat/chat-input-bar.react.js
380–382

Is there a reason these are imported and just passed in as props? Can they be imported directly in typehead-tooltip.react.js?

web/chat/typeahead-tooltip.react.js
28–33

If you can just import them directly here, that'd remove the complexity in the props here.

This revision now requires changes to proceed.Tue, Aug 29, 10:31 AM
patryk added inline comments.Wed, Aug 30, 12:25 AM
web/chat/chat-input-bar.react.js
380–382

This is because in the future, we might want to create another typeahead tooltip that renders buttons with a different style. I have provided an explanation for why it's worth considering this refactoring here.

rohan accepted this revision.Wed, Aug 30, 10:23 AM
rohan added inline comments.
web/chat/chat-input-bar.react.js
380–382

Ah ok I guess it makes sense to make future work easier

This revision is now accepted and ready to land.Wed, Aug 30, 10:23 AM

Revision Contents
Changeset List

PathSize
web/
chat/
47 lines
52 lines
utils/
107 lines

Diff 30307

View Options

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

Loading...
View Options

web/chat/typeahead-tooltip.react.js

Loading...
View Options

web/utils/typeahead-utils.js

Loading...