Page MenuHomePhabricator

[web] Move logic for fetching ThreadInfo for possibly pending chat out of ChatMessageListContainer component
ClosedPublic

Authored by inka on Feb 15 2023, 5:32 AM.
Tags
None
Referenced Files
Unknown Object (File)
Mon, Mar 25, 2:07 PM
Unknown Object (File)
Mon, Mar 25, 2:07 PM
Unknown Object (File)
Mon, Mar 25, 2:06 PM
Unknown Object (File)
Mon, Mar 25, 2:06 PM
Unknown Object (File)
Mon, Mar 25, 2:06 PM
Unknown Object (File)
Mon, Mar 25, 2:06 PM
Unknown Object (File)
Mon, Mar 25, 2:06 PM
Unknown Object (File)
Mon, Mar 25, 2:05 PM
Subscribers

Details

Summary

Issue: https://linear.app/comm/issue/ENG-2740/add-navigation-state-info-to-the-top-bar
The new designs call for the navigation state info to be moved from message list header, to main content's header. The component that is used for displaying the nav state info ('ThreadTopBar') needs the ThreadInfo of the open chat, or
a special ThreadInfo object when a chat is being created. The excrated logic is used to obtain that exact ThreadInfo. This will allow me to use it in the Topbar component, where I'll need to display the nav state info.

Test Plan

Run web app, check that the ThreadTopBar component displys correctly in the message list header

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

inka requested review of this revision.Feb 15 2023, 5:47 AM

MAke useThreadInfoForPossiblyPendingThread return an optional value

web/chat/chat-message-list-container.react.js
46–47 ↗(On Diff #22619)

Looks like this one is unused right now

web/utils/thread-utils.js
13 ↗(On Diff #22619)

can be merged with line 12

18–22 ↗(On Diff #22619)

it's a hook with one prop so maybe there is no need for an object, and you can use simply one function's argument

25–32 ↗(On Diff #22619)

This is a lot of code repetition, I'm wondering if it will not be better to create a selector which returns all these fields

web/chat/chat-message-list-container.react.js
46–47 ↗(On Diff #22619)

Oh, haha, I relied on my IDE underlining things that are unused, but since it's checked in the invariant... 🤦‍♀️

web/utils/thread-utils.js
17–22 ↗(On Diff #22666)

All of this data is only used when a thread is pending

Make activeChatThreadID optional in useThreadInfoForPossiblyPendingThread. The code inside useThreadInfoForPossiblyPendingThread handles this anyway, and it allows to avoid ugly code where this function is used in future diffs
(assigning empty
strings to activeChatThreadID, and such)

This revision is now accepted and ready to land.Feb 22 2023, 3:28 AM