issue: https://linear.app/comm/issue/ENG-4163/add-results-and-position-to-message-search-context
If the user searches for something, closes the modal and reopens it, we want to show them the results they have previously fetched. We only want the client to call the server, if the user presses the
"Search" button, or scrolls to the bottom.
This required a lot of changes, because we are no longer relying on useEffects to fetch messages. Instead we do it manually in onPressSearch and onScroll(-> possiblyLoadMoreMessages), and make
sure to not fetch the same results twice by using the loading ref.
Details
Details
- Reviewers
kamil • kuba michal - Commits
- rCOMM5204fff125b1: [web] Remember resutls in search modal
Checked that the results are remembered and displayed after reopening the modal, without refetching. Checked that if a user scrolls down or presses "search", new results are fetched.
Diff Detail
Diff Detail
- Repository
- rCOMM Comm
- Lint
No Lint Coverage - Unit
No Test Coverage
Event Timeline
| web/modals/search/message-search-modal.react.js | ||
|---|---|---|
| 59 ↗ | (On Diff #28041) | I'm wondering if it would make sense to move useSearchMessages to the context and then expose search messages with "curried" getQuery() and threadInfo.id (and part of appendResults). But not sure how that would interact with loading and lastID |
| 62 ↗ | (On Diff #28041) | I think this can be inlined |
| 137 ↗ | (On Diff #28041) | I don't think this will work. getQuery uses a ref inside and so won't get recreated when query.current changes. It might work because other dependencies will also change but I'm not sure. For example you delete the query (with backspace) will the footer update? |
| web/modals/search/message-search-modal.react.js | ||
|---|---|---|
| 59 ↗ | (On Diff #28041) | I will address this in the next iteration of answering to review. I will first address the easy stuff, and then try to rewrite the code in this way |
| 137 ↗ | (On Diff #28041) | It's not supposed to change when I delete the query with backspace as far as I understand, but this ties back to your comment on D8274, and is being discussed in ENG-3466 The footer is reloaded when the query is "cleared and saved", because the modifiedItems.length changes, and then it checks the query to determine what the footer should be |
| web/modals/search/message-search-modal.react.js | ||
|---|---|---|
| 137 ↗ | (On Diff #28041) | Oh right, sorry I got confused, with the search button, changing the query isn't supposed to update the footer anyway. |
| web/modals/search/message-search-modal.react.js | ||
|---|---|---|
| 101 | This is a boolean, so it will only cause a rerender if the value changes. We need it's change to cause the rerender of the footer, in case no results were returned. In other cases the rerender is caused by modifiedItems.length changing | |
| web/modals/search/message-search-modal.react.js | ||
|---|---|---|
| 102 ↗ | (On Diff #28161) | This is needed for the case when the results have not yet been fetched but the query is cleared |