Page MenuHomePhabricator

[web] Render `ChatMessageList` only if `activeChatThreadID` exists
ClosedPublic

Authored by jacek on Aug 1 2022, 6:31 AM.
Tags
None
Referenced Files
F3302748: D4692.diff
Mon, Nov 18, 8:23 AM
Unknown Object (File)
Sat, Nov 9, 11:38 AM
Unknown Object (File)
Fri, Nov 8, 8:36 PM
Unknown Object (File)
Fri, Nov 8, 8:36 PM
Unknown Object (File)
Tue, Nov 5, 2:06 AM
Unknown Object (File)
Sun, Oct 20, 2:47 PM
Unknown Object (File)
Oct 2 2024, 1:36 PM
Unknown Object (File)
Sep 26 2024, 6:08 PM
Subscribers

Details

Summary

Fix for issue: https://linear.app/comm/issue/ENG-1489/web-app-was-not-loading-properly-error-with-chatmessagelistcontainer
Moved activeChatThreadID selector one level higher in components tree to conditionally render ChatMessageListContainer and avoid need to further modifications inside, as the component was designed with assumption that activeChatThreadID always exists.

Test Plan

Register new user in app. Don't enter any thread on mobile. After opening chat in web app, the issue described in Linear task no longer appears.

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

tomek added inline comments.
web/chat/chat.react.js
14–21 ↗(On Diff #15133)

What is the reason behind introducing this memo?

This revision is now accepted and ready to land.Aug 1 2022, 6:48 AM
web/chat/chat.react.js
14–21 ↗(On Diff #15133)

To avoid re-rendering ThreadListProvider when activeChatThreadID changes, as the Chat component will be re-rendered more often now. (Although I suppose the ThreadListProvider would also be re-rendered even inside Memo, as it also uses activeChatThreadID.)

This makes sense. Thanks for being thorough in your logic in the Linear issue, made this diff easier to review.