Page MenuHomePhabricator

[lib] Make `useInlineSidebarText` hook parameter optional
ClosedPublic

Authored by jacek on Aug 12 2022, 2:19 AM.
Tags
None
Referenced Files
Unknown Object (File)
Mon, Nov 18, 3:16 PM
Unknown Object (File)
Sat, Nov 9, 11:47 PM
Unknown Object (File)
Sat, Nov 9, 8:43 PM
Unknown Object (File)
Sat, Nov 9, 8:37 PM
Unknown Object (File)
Sat, Nov 9, 7:38 PM
Unknown Object (File)
Sat, Nov 9, 5:43 PM
Unknown Object (File)
Sat, Nov 9, 3:46 PM
Unknown Object (File)
Fri, Nov 8, 5:32 AM
Subscribers

Details

Summary

When we add reactions to InlineSidebar component we want to support the case, when there are reactions and there is no existing thread from the message.
In that case, we don't have ThreadInfo. The idea is to make the hook return empty strings when no there is threadInfo provided. It will help avoid conditional execution of the hook in that case.

Test Plan

Current behavior remain unchaged. After introducing InlineSidebar redesign, confirmed that the hook returns empty result strings when there is no thread provided.

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

jacek held this revision as a draft.
tomek requested changes to this revision.Aug 12 2022, 5:32 AM
tomek added inline comments.
lib/hooks/inline-sidebar-text.react.js
17–20 ↗(On Diff #15596)

The current code is confusing because we set a number of replies to be 1 when a thread doesn't exist, then determine repliesText based on it, just to ignore it later. We can avoid this issue by moving repliesCount and repliesText into the last memo and early exit from it when !threadInfo.

This revision now requires changes to proceed.Aug 12 2022, 5:32 AM
lib/hooks/inline-sidebar-text.react.js
17–20 ↗(On Diff #15596)

Totally agree. I'll move it.

Other than what @tomek said, this looks good. So if I understand correctly, threadInfo can now be null, undefined, or void, and in that case, we will return empty strings to InlineSidebar, since we still want to display the reactions but no text for a thread (since it doesn't exist)?

tomek added inline comments.
lib/hooks/inline-sidebar-text.react.js
31

At this point you should be able to use threadInfo

40

If you feel like it, you can optimize this a bit as it only depends on !threadInfo and threadInfo?.repliesCount. That would require introducing a new variable for !threadInfo.

This revision is now accepted and ready to land.Aug 16 2022, 8:41 AM