[web] implemented search feature for Thread Picker
ClosedPublic
Actions

Authored by ginsu on Sep 27 2022, 8:35 AM.

Details

Reviewers

atul
• abosh
rohan
tomek

Commits

rCOMM3ea219748a79: [web] implemented search feature for Thread Picker

Summary

Implemented the logic for the search feature for the ThreadPicker component. There is no UI for this diff due to Search breaking the app since the focusing/blurring of the picker component prevents input in Search

ENG-1847

Depends on D5230

Test Plan

To test the search feature, please add and modify the following blocks of code:

Screen Shot 2022-09-27 at 11.32.03 AM.png (444×952 px, 79 KB)

let searchBar = null;
if (this.props.searchIndex) {
  searchBar = (
    <Search
      searchText={this.state.searchText}
      onChangeText={this.onChangeSearchText}
      placeholder="Search chats"
    />
  );
}

Screen Shot 2022-09-27 at 11.27.40 AM.png (624×1 px, 96 KB)

You will also need to import the Search component from ../components/search.react at the top of the file

Once you have the Search component being returned you can add a new entry and test that the search feature is working as expected

Diff Detail

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

ginsu created this revision.Sep 27 2022, 8:35 AM

Herald added subscribers: • abosh, atul, tomek, ashoat. · View Herald TranscriptSep 27 2022, 8:35 AM

ginsu edited the summary of this revision. (Show Details)Sep 27 2022, 8:45 AM

ginsu edited the test plan for this revision. (Show Details)

ginsu added reviewers: atul, • abosh, rohan.

ginsu added a parent revision: D5230: [web] removed pagination logic from thread picker.

ginsu attached a referenced file: F183591: Screen Shot 2022-09-27 at 11.27.40 AM.png. (Show Details)

ginsu attached a referenced file: F183590: Screen Shot 2022-09-27 at 11.32.03 AM.png. (Show Details)

Harbormaster completed remote builds in B12465: Diff 17112.Sep 27 2022, 8:46 AM

ginsu requested review of this revision.Sep 27 2022, 8:46 AM

atul added a reviewer: tomek.Sep 28 2022, 1:10 PM

made searchIndex a requried prop

ginsu added inline comments.Sep 28 2022, 2:40 PM

web/calendar/thread-picker.react.js
73–89 ↗	(On Diff #17161)	This logic comes from the native ThreadList component

Harbormaster completed remote builds in B12504: Diff 17161.Sep 28 2022, 2:47 PM

atul added 1 blocking reviewer(s): atul.Sep 29 2022, 6:56 AM

atul accepted this revision.Sep 29 2022, 8:18 AM

This revision is now accepted and ready to land.Sep 29 2022, 8:18 AM

atul added inline comments.Sep 29 2022, 8:19 AM

web/calendar/thread-picker.react.js
73–89 ↗	(On Diff #17161)	Thanks for linking to that!

ashoat added inline comments.Sep 29 2022, 9:19 AM

web/calendar/thread-picker.react.js
73–89 ↗	(On Diff #17161)	Ah yeah this is shitty. I understand now why you guys created ENG-1914. Options here: Leave as-is (probably fine) Move whole thing to functional component (also probably fine) Extract `searchText` and `searchResults` state into `ConnectedThreadPicker`, pass getters/setters into class component. Then you can determine `listData` in `ConnectedThreadPicker` using `React.useMemo` Separate note – if ENG-1914 had the right context of why we wanted to switch to a functional component (not that it needs to be "modern", but that we need to memoize something that depends on state) then I wouldn't have pushed back. Anyways, feel free to reopen ENG-1914 if you guys prefer! Or leave as-is. I think any of the solutions above would be fine.