issue: https://linear.app/comm/issue/ENG-3466/add-some-state-for-keeping-the-search-query
We want to be able to remeber the query the user has typed in, for each chat.
Details
Details
- Reviewers
kamil • kuba michal tomek - Commits
- rCOMMafc66dd9e6b8: [web] Add message search context
Tested with subsequent diffs
Diff Detail
Diff Detail
- Repository
- rCOMM Comm
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
Comment Actions
Add clearQuery
| web/search/message-search-state-provider.react.js | ||
|---|---|---|
| 38–41 ↗ | (On Diff #27926) | Not sure if there is a better way of doing this? |
| web/search/message-search-state-provider.react.js | ||
|---|---|---|
| 38–41 ↗ | (On Diff #27926) | The current implementation mutates the state. If there would be no better solution, we should do something like: setQueries(prevQueries => {
const newState = {...prevQueries};
delete newState[threadID];
return newState;
}),but there's a better solution setQueries(prevQueries => {
const {[threadID]:deleted, ...newState} = prevQueries;
return newState;
}),(but Flow might complain about unused variable deleted) |
| web/search/message-search-state-provider.react.js | ||
|---|---|---|
| 26 ↗ | (On Diff #27929) | What do you think about changing the approach so that a thread id is a parameter of set query? |
| 28 ↗ | (On Diff #27929) | Where this context will be rendered? Are we sure that this will always be true? Can we avoid having this invariant? |
| web/search/message-search-state-provider.react.js | ||
|---|---|---|
| 26 ↗ | (On Diff #27929) | Why? I thought this was a way better approach because the calling code doesn't have to do this, it's abstracted away. |