to make search sidebars work with the SearchModal component it's state needs to be refactored to just the result set. This diff removes the text key/value and moves it into a React.useState which is later refactored in the following diff to get sidebar search data to work with SearchModal component.
Details
Details
- Reviewers
atul • jacek • benschac
Flow and eslint don't error. The sidebar search isn't functional in this diff but will be in the next diff.
Diff Detail
Diff Detail
- Repository
- rCOMM Comm
- Lint
No Lint Coverage - Unit
No Test Coverage
Event Timeline
| lib/hooks/search-sidebars.js | ||
|---|---|---|
| 91 ↗ | (On Diff #12775) | After implementing <SearchModal /> in the following diff, we no longer need clearQuery and it's removed in the next diff. |
| lib/hooks/search-sidebars.js | ||
|---|---|---|
| 15 ↗ | (On Diff #12826) | Why do we repeat text? |
| 58 ↗ | (On Diff #12826) | We don't usually start variable names with onSomething - this name is used for callbacks. |
| native/chat/sidebar-list-modal.react.js | ||
| 59 ↗ | (On Diff #12826) | I have some doubts about it. Currently, all the clients of the hook can set both text and search result. It would be a lot easier to reason about the state if only the text could be set and then the result would be updated by the hook. Do you think this can be achieved? |
Comment Actions
Later in the stack, it is mentioned that this is a part of https://linear.app/comm/issue/ENG-536/sidebar-list-modal-re-style - which is marked as done. If we decide to do something more about that issue, it would be probably easier to do it from scratch instead of figuring out how to make this stack solve the problem.