Page MenuHomePhabricator

[web] Keep `activeChatThreadID` and `pendingThread` updated in `navInfo` in thread creation mode
ClosedPublic

Authored by jacek on Jul 7 2022, 6:39 AM.
Tags
None
Referenced Files
Unknown Object (File)
Sat, Nov 16, 11:46 PM
Unknown Object (File)
Wed, Nov 13, 4:39 AM
Unknown Object (File)
Fri, Nov 8, 4:00 PM
Unknown Object (File)
Fri, Nov 8, 1:38 PM
Unknown Object (File)
Fri, Nov 8, 1:37 PM
Unknown Object (File)
Fri, Nov 8, 12:57 PM
Unknown Object (File)
Fri, Nov 8, 3:44 AM
Unknown Object (File)
Fri, Nov 8, 3:27 AM

Details

Summary

The diff introduces updating activeChatThreadID and pendingThread in navInfo in thread creation mode.
These values from navInfo are not used by chat-message-list-container when creating thread, but can be read from different places of the app - e.g. by ThreadListProvider.
After the change, thread currently being created (or selected from existing by matching users) is correctly displayed in the thread list on the left, and after clicking on the thread on list, the creation mode is closed and pending thread is correctly being displayed.

Test Plan

Selecting some new set of users in thread creation mode to display pending thread. It should be correclty displayed on the top of the thread list.
After clicking on the thread, creation mode should be closed and pending thread should remain open (even after page refresh)

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

tomek added inline comments.
web/chat/chat-message-list-container.react.js
114 ↗(On Diff #14265)

Is it possible to avoid having an effect and

  1. Set correct nav info when navigating

or

  1. Determine pendingThread in a reducer?
134–135 ↗(On Diff #14265)

We can merge these

This revision is now accepted and ready to land.Jul 20 2022, 5:07 AM

changed the types to () => mixed & wrap using useCallback
Accidentally added description from different diff

web/chat/chat-message-list-container.react.js
114 ↗(On Diff #14265)

We update navInfo hare with data calculated by existingThreadInfoFinder, so to handle navigation update in reducer we'd need to move all the logic responsible for finding thread to reducer.
Atul suggested in D4455 that we should keep nav-reducer possibly clean and clear, and moving the thread selection logic there would definitely make reducer really complex.

Determining pending thread in reducer basing on activeChatThreadID is possible, and was introduced in the first iteration of D4455, but we decided to move it out of reducer, to keep reducer clear. However, finding proper activeChatThreadIDthere is quite complex (using existingThreadInfoFinder).

This revision is now accepted and ready to land.Jul 21 2022, 6:31 AM