[native] Forward `searchInputRef` from `ChatThreadList` to `ChatThreadListSearch`
ClosedPublic
Actions

Authored by atul on Sep 14 2023, 2:05 PM.

Details

Reviewers

tomek
ginsu
rohan

Commits

rCOMMb2d1a3f8e4f9: [native] Forward `searchInputRef` from `ChatThreadList` to…

Summary

Forward ref from ChatThreadList to ChatThreadListSearch... which will in turn forward it to the inner Search component.

Depends on D9208

Test Plan

Search experience continues to look and work as expected. Set logs in all expected callbacks to ensure that they fired as expected. Made sure that x-icon and "Cancel" button continue to look and behave as before.

Diff Detail

Repository

rCOMM Comm

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

atul created this revision.Sep 14 2023, 2:05 PM

Herald added a subscriber: ashoat. · View Herald TranscriptSep 14 2023, 2:05 PM

atul published this revision for review.Sep 14 2023, 2:06 PM

atul added a child revision: D9210: [native] Add some basic memoization to `ChatThreadListSearch`.Sep 14 2023, 2:16 PM

Harbormaster completed remote builds in B22644: Diff 31155.Sep 14 2023, 2:24 PM

tomek accepted this revision.Sep 18 2023, 4:13 AM

tomek added inline comments.

native/chat/chat-thread-list-search.react.js
81 ↗	(On Diff #31155)	Why this ref was defined here and used only as a prop?
native/chat/chat-thread-list.react.js
356 ↗	(On Diff #31155)	Are we sure that we're rendering at most one instance of this component? I'm wondering if using the same ref for two instances can cause some issues.

This revision is now accepted and ready to land.Sep 18 2023, 4:13 AM

atul added inline comments.Sep 18 2023, 10:27 AM

native/chat/chat-thread-list-search.react.js
81 ↗	(On Diff #31155)	This was temporary to appease flow, etc. What we really want to do (and do in this diff) is forward ref from `ChatThreadList` down to the inner `Search` component.
native/chat/chat-thread-list.react.js
356 ↗	(On Diff #31155)	I believe it should be fine. This matches how things were previously.