Page MenuHomePhabricator
Differential D7739

[native] Add message search context
ClosedPublic
Actions

Authored by inka on May 8 2023, 4:19 AM.
Tags
None
Referenced Files
F3367856: D7739.diff
Mon, Nov 25, 4:45 PM
Unknown Object (File)
Fri, Nov 8, 1:16 AM
Unknown Object (File)
Thu, Nov 7, 11:45 PM
Unknown Object (File)
Thu, Nov 7, 10:38 PM
Unknown Object (File)
Thu, Nov 7, 6:41 PM
Unknown Object (File)
Thu, Nov 7, 5:15 PM
Unknown Object (File)
Thu, Nov 7, 5:15 PM
Unknown Object (File)
Thu, Nov 7, 4:23 PM
View All 100 Files
Subscribers
ashoat
atul
tomek

Details

Reviewers
• kuba
kamil
bartek
Commits
rCOMM1d14e082facb: [native] Add message search context
Summary

issue: https://linear.app/comm/issue/ENG-3163/create-the-search-screen-for-message-searching
Since the query has to be passed between the search screen and the header component, that is passed in screen options, I created a context to hold the query

Test Plan

tested with next diffs

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

inka created this revision.May 8 2023, 4:19 AM
Herald added subscribers: atul, tomek, ashoat. · View Herald TranscriptMay 8 2023, 4:19 AM
inka added a parent revision: D7652: [lib] Allow calling message search from client.May 8 2023, 4:20 AM
inka added a child revision: D7740: [native] Add a header componnent with a search bar.May 8 2023, 4:24 AM
Harbormaster completed remote builds in B19124: Diff 26178.May 8 2023, 4:35 AM
inka requested review of this revision.May 8 2023, 4:35 AM
bartek accepted this revision.May 9 2023, 2:59 AM
This revision is now accepted and ready to land.May 9 2023, 2:59 AM
kamil accepted this revision.May 10 2023, 9:40 AM
kamil added inline comments.
native/search/search-provider.react.js
2 ↗(On Diff #26178)

I think you should add new line here

6 ↗(On Diff #26178)

do we consume it somewhere, so that there is a need to export?

21 ↗(On Diff #26178)

personally, I prefer to infer the type from the default argument as for me this redundancy - but this is frequent in the codebase so as you prefer

inka added inline comments.May 20 2023, 12:38 AM
native/search/search-provider.react.js
6 ↗(On Diff #26178)

No, but even if we don't use it anywhere else we usually export ContexType from a file

inka updated this revision to Diff 26716.May 20 2023, 12:39 AM

Address review

Harbormaster completed remote builds in B19536: Diff 26716.May 20 2023, 12:57 AM
Closed by commit rCOMM1d14e082facb: [native] Add message search context (authored by • InkaSokolowska). · Explain WhyMay 24 2023, 2:12 AM
This revision was automatically updated to reflect the committed changes.
• InkaSokolowska added a commit: rCOMM1d14e082facb: [native] Add message search context.

Revision Contents
Changeset List

PathSize
native/
29 lines
search/
42 lines

Diff 26716

View Options

native/root.react.js

Loading...
View Options

native/search/search-provider.react.js

Loading...