Linear issue: https://linear.app/comm/issue/ENG-2650/make-the-calendar-change-on-community-drawer-action
Pressing an item in the community drawer on web should result in different behaviour depending on the app the user has currently open. In chat it is supposed to open the corresponding chat. In calendar
it is supposed to update the filters so that only the events of the corresponding chat are displayed. In Apps it is supposed to navigate back to the Chat tab and open the corresponding chat.
For each App we either implement a Handler component, or it'll use the default Handler. We obtain these Handlers via getCommunityDrawerItemHandler function. The Handler component doesn't
render anything, but sets a handler object in the drawer item via setHandler. This handler object is of CommunityDrawerItemHandlerSpec type, so it implements all functions that are needed by
the drawer in each tab (that are specific to the tab). For now, it implements the "onClick" function.
Details
Run web app, check that pressing items in the drawer results in the behaviour I described above, based on the tab the user is in.
Diff Detail
- Repository
- rCOMM Comm
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
web/sidebar/community-drawer-item.react.js | ||
---|---|---|
73–76 ↗ | (On Diff #21074) | We don't need to memoize the Handler because it can only return a couple of const values which aren't created inside getCommunityDrawerItemHandler. But this is an implementation detail of getCommunityDrawerItemHandler and relying on that has a tiny chance of causing issues in the future. A better idea might be to call this function as a part of the selector. This has an additional benefit of a possible performance optimization because there are multiple tabs for which the same handler value is provided. |
79–80 ↗ | (On Diff #21074) | Why do we need to silence this message? Would removing a parameter solve the issue? |
85 ↗ | (On Diff #21074) | Are we sure that these conditions remain correct for all the other apps? What happens when we are creating a chat and then decide to open a calendar? Will the drawer be useful? |
web/sidebar/community-drawer-item.react.js | ||
---|---|---|
79–80 ↗ | (On Diff #21074) | If I do that Flow doesn't allow me to use this function in line 86, since the function does not expect an argument and i'm passing it an event |
85 ↗ | (On Diff #21074) | I started to think about this, and actually here is what happens for ChatThreadList, where I took this code from: I suppose this is a bug? Anyway this condition means that we cannot navigate to a thread if it is both active and in create mode. |
web/sidebar/community-drawer-item.react.js | ||
---|---|---|
91 ↗ | (On Diff #21146) | When the onClick callback is so simple, there's no need to create it at all. |
85 ↗ | (On Diff #21074) | Yes, it is a bug - could you create a task for it? Regarding the additional dispatch, if it doesn't affect the history, it doesn't matter that much. But it might be a good idea to make the current thread unclickable. |
web/sidebar/community-drawer-item.react.js | ||
---|---|---|
85 ↗ | (On Diff #21074) | [[ https://linear.app/comm/issue/ENG-2824/[web]-changing-the-tab-while-creating-a-new-chat-results-in-weird | issue ]], (I think the [web] in url is breaking the formatting on phab) |