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.
Details
Details
Current behavior remain unchaged. After introducing InlineSidebar redesign, confirmed that the hook returns empty result strings when there is no thread provided.
Diff Detail
Diff Detail
- Repository
- rCOMM Comm
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
| 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. |
| lib/hooks/inline-sidebar-text.react.js | ||
|---|---|---|
| 17–20 ↗ | (On Diff #15596) | Totally agree. I'll move it. |
Comment Actions
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)?
| lib/hooks/inline-sidebar-text.react.js | ||
|---|---|---|
| 31 ↗ | (On Diff #15651) | At this point you should be able to use threadInfo |
| 40 ↗ | (On Diff #15651) | 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. |