Page MenuHomePhabricator

[native] Flatten nested FlatLists in community drawer
ClosedPublic

Authored by inka on Jun 6 2023, 8:32 AM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Apr 19, 12:18 AM
Unknown Object (File)
Thu, Apr 18, 4:04 AM
Unknown Object (File)
Thu, Apr 18, 4:04 AM
Unknown Object (File)
Thu, Apr 18, 4:04 AM
Unknown Object (File)
Thu, Apr 18, 4:04 AM
Unknown Object (File)
Thu, Apr 18, 4:04 AM
Unknown Object (File)
Thu, Apr 18, 3:59 AM
Unknown Object (File)
Mar 5 2024, 11:53 PM
Subscribers

Details

Summary

issue: https://linear.app/comm/issue/ENG-3933/flatten-the-list-of-items-to-render-in-communitydrawercontent
CommunityDrawerItemCommunity and CommunityDrawerItemChat are no longer needed:

CommunityDrawerItemCommunity existed mostly to take care of some styles, but because of changes needed to remove nested FlatLists, the styles are now handled differently - by the itemData.itemStyle prop.

CommunityDrawerItemChat existed to hold the expanded state for items on deeper levels, but this state (as discussed in ENG-3934 ) is now handled by CommunityDrawerContent.

Because the expanded state for all items is now handled by CommunityDrawerContent, openCommunity state is no longer needed. We can check which community is open by expanded.includes(id). expanded initial state is communitiesSuffixed.length === 1 ? [communitiesSuffixed[0].id] : [], because if there is only one community it is supposed to be open by default. Communnity items still need a different function for toggleExpanded though, because when one community is opened, the other one collapses automatically.

Test Plan

Checked that the drawer bahaves as before:

  • only one community is open at a time
  • chats on lower levels can be open simultaneously
  • collapsing a chat causes all its children to collapse
  • items are styled properly

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

inka requested review of this revision.Jun 6 2023, 8:49 AM
tomek requested changes to this revision.Jun 7 2023, 5:31 AM
tomek added inline comments.
native/navigation/community-drawer-content.react.js
68–70 ↗(On Diff #27475)

We should use Set instead so that checking if it includes an id is more efficient

117–119 ↗(On Diff #27475)
native/navigation/community-drawer-item.react.js
21 ↗(On Diff #27475)

With changes from this diff now it becomes a little confusing that expanded might be list / set or a flag, depending on the context. Maybe rename it to isExpanded` here?

91 ↗(On Diff #27475)

We should be as specific as possible when defining the dependencies

This revision now requires changes to proceed.Jun 7 2023, 5:31 AM
tomek added inline comments.
native/navigation/community-drawer-content.react.js
105–120 ↗(On Diff #27522)

Why do we use setState(oldState => newState) in just one branch? Also, checking if condition should also happen inside set state function.

This revision is now accepted and ready to land.Jun 12 2023, 3:51 AM