Page MenuHomePhabricator

[lib, native] Extract from `useSearchSidebars` code that can be used to obtain chats subchannels info.
ClosedPublic

Authored by inka on Dec 20 2022, 3:34 AM.
Tags
None
Referenced Files
Unknown Object (File)
Sat, Nov 23, 1:55 PM
Unknown Object (File)
Sat, Nov 23, 1:43 PM
Unknown Object (File)
Sat, Nov 23, 1:33 PM
Unknown Object (File)
Sat, Nov 23, 11:37 AM
Unknown Object (File)
Thu, Nov 21, 10:14 PM
Unknown Object (File)
Sun, Nov 10, 4:13 PM
Unknown Object (File)
Fri, Nov 8, 10:15 AM
Unknown Object (File)
Fri, Nov 8, 6:55 AM
Subscribers

Details

Summary

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.

Test Plan

Run the ios simulator, see that sidebar modals display correctly.
Tested the useSearchSubchannels by logging to the console what it returned.

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

inka requested review of this revision.Dec 20 2022, 3:46 AM
inka planned changes to this revision.Dec 20 2022, 6:42 AM

With what the designs specify it seems I will need to change what I use in the drawer

Add changes allowing to display las sent message for items in subchannels modal

inka edited the summary of this revision. (Show Details)
inka edited the test plan for this revision. (Show Details)

Change variable name

Use sidebarInfoSelector in useSearchSidebars

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).

inka planned changes to this revision.Dec 21 2022, 5:13 AM

Turned out that I didn't need to use sidebarInfoSelector in my use case, so reverting unnecessary changes

inka retitled this revision from [lib, native] Reaname and generalize some types and selectors to allow their reuse to [lib, native] Extract from `useSearchSidebars` code that can be used to obtain chats subchannels info. .Dec 21 2022, 6:42 AM
inka edited the summary of this revision. (Show Details)
inka edited the test plan for this revision. (Show Details)
kamil added a reviewer: tomek.
kamil added inline comments.
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.
Maybe it is possible to create a generic type for this? Or if it will cause problems creating common base attributes as separate type and then spreading it here.

tomek requested changes to this revision.Dec 23 2022, 3:09 AM

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.

This revision now requires changes to proceed.Dec 23 2022, 3:09 AM
inka retitled this revision from [lib, native] Extract from `useSearchSidebars` code that can be used to obtain chats subchannels info. to [lib, native] Extract from `useSearchSidebars` code that can be used to obtain chats subchannels info..
inka edited the summary of this revision. (Show Details)

Address code review

This revision is now accepted and ready to land.Dec 27 2022, 4:47 AM