Community drawer has buttons that navigate to a view of given chats subchannels. As discussed in https://linear.app/comm/issue/DES-20/designs-for-subchannels-modal, https://linear.app/comm/issue/ENG-2373/subchannels-view this view will display items that look similar to items in SidebarListModal, so I will want to reuse a lot of the code. I extracted from code that was used to obtain chats sidebars info code that can be used to obtain chats subchannels and made wrapper functions for these two use cases.
Details
Details
Run the ios simulator, see that sidebar modals display correctly.
Tested the useSearchSubchannels by logging to the console what it returned.
Diff Detail
Diff Detail
- Repository
- rCOMM Comm
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
Comment Actions
With what the designs specify it seems I will need to change what I use in the drawer
Comment Actions
Before, useSearchSidebars obtained the children data (sidebarInfos, renamed in this diff to childThreadInfos) by directly calling the sidebarInfoSelector. Because I wanted to reuse most of the useSearchSidebars, I renamed the function to useSearchThreads. In my use case the children data is different, so now it is passed to this function as an argument. Fetching this data is now handled by wrapper functions: useSearchSidebars, that fetches it the same way it was done before (using sidebarInfoSelector), and useSearchSubchannels that fetches the data needed for my use case - getting the subchannels (using useFilteredChatListData).
Comment Actions
Turned out that I didn't need to use sidebarInfoSelector in my use case, so reverting unnecessary changes
| lib/hooks/search-threads.js | ||
|---|---|---|
| 5–6 ↗ | (On Diff #19947) | can be merged |
| 11–16 ↗ | (On Diff #19947) | this also can be merged |
| 118–122 ↗ | (On Diff #19947) | This signature is used three times in this file, and what is more, they're connected logically (e.g. here this is basically the same as what will be returned by useSearchThreads), and the only difference is listData type. |
Comment Actions
Usually when moving and modifying the code, we should od that separately https://www.notion.so/commapp/Moving-code-around-bbb551c4350b4d5cb553c6751be44314 so the review is easier. In this case the code wasn't exactly moved - only a file was renamed, and it makes a review only slightly harder, so we can keep it.
| lib/hooks/search-threads.js | ||
|---|---|---|
| 110–111 ↗ | (On Diff #19947) | In the original code we were getting a value inside the selector. It makes a big difference in performance, because we would like to render our component when sidebarInfoSelector(state)[threadInfo.id] changes and not when the whole sidebarInfoSelector(state) does that. |
| 125–127 ↗ | (On Diff #19947) | We should use our existing logic. In this case it seems like we can use threadIsChannel function. |