Page MenuHomePhabricator
Differential D9208

[native] Remove `renderSearch` in `ChatThreadList` and inline `ChatThreadListSearch` directly
ClosedPublic
Actions

Authored by atul on Thu, Sep 14, 1:41 PM.
Tags
None
Referenced Files
F772791: D9208.diff
Mon, Sep 25, 2:36 AM
Unknown Object (File)
Fri, Sep 22, 4:19 PM
Subscribers
ashoat

Details

Reviewers
ginsu
tomek
rohan
Commits
rCOMMa0967233d046: [native] Remove `renderSearch` in `ChatThreadList` and inline…
Summary

Instead of passing a function to create ChatThreadListSearch we can inline the component directly.

As part of this we also replace additionalProps w/ more precise innerSearchAutoFocus and innerSearchActive props.

Depends on D9207

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.Thu, Sep 14, 1:41 PM
Herald added a subscriber: ashoat. · View Herald TranscriptThu, Sep 14, 1:41 PM
atul published this revision for review.Thu, Sep 14, 1:41 PM
atul updated this revision to Diff 31154.Thu, Sep 14, 1:43 PM

some memoization

Harbormaster completed remote builds in B22642: Diff 31153.Thu, Sep 14, 1:59 PM
Harbormaster completed remote builds in B22643: Diff 31154.Thu, Sep 14, 2:03 PM
atul added a child revision: D9209: [native] Forward `searchInputRef` from `ChatThreadList` to `ChatThreadListSearch`.Thu, Sep 14, 2:05 PM
tomek accepted this revision.Mon, Sep 18, 3:58 AM
tomek added inline comments.
native/chat/chat-thread-list.react.js
361 ↗(On Diff #31154)

We can use a shorthand

364 ↗(On Diff #31154)

We can also consider having an early return

This revision is now accepted and ready to land.Mon, Sep 18, 3:58 AM
atul updated this revision to Diff 31212.Mon, Sep 18, 10:22 AM

rebase

atul added inline comments.Mon, Sep 18, 10:25 AM
native/chat/chat-thread-list.react.js
361 ↗(On Diff #31154)

I'd prefer to be explicit here

364 ↗(On Diff #31154)

Will update to early return

atul updated this revision to Diff 31213.Mon, Sep 18, 10:25 AM

address feedback: early-return

atul updated this revision to Diff 31214.Mon, Sep 18, 10:28 AM

rebase

Harbormaster completed remote builds in B22684: Diff 31212.Mon, Sep 18, 10:40 AM
Harbormaster completed remote builds in B22685: Diff 31213.Mon, Sep 18, 10:42 AM
Harbormaster completed remote builds in B22686: Diff 31214.Mon, Sep 18, 10:45 AM
Closed by commit rCOMMa0967233d046: [native] Remove `renderSearch` in `ChatThreadList` and inline… (authored by atul). · Explain WhyMon, Sep 18, 11:00 AM
This revision was automatically updated to reflect the committed changes.
atul added a commit: rCOMMa0967233d046: [native] Remove `renderSearch` in `ChatThreadList` and inline….

Revision Contents
Changeset List

Diff 31213

View Options

native/chat/chat-thread-list-search.react.js

Loading...
View Options

native/chat/chat-thread-list.react.js

Loading...