[lib] [refactor] [ENG-530] add memo in inline sidebar hook
ClosedPublic
Actions

Authored by • benschac on May 2 2022, 8:25 AM.

Details

Reviewers

atul
• jacek
• abosh
ashoat

Commits

rCOMM80abee4eb84a: [lib] [refactor] [ENG-530] add memo in inline sidebar hook

Summary

updating the hook per my comment in my last diff: https://phabricator.ashoat.com/D3879#inline-23803

Test Plan

use the application like normal. replied should reflect in a sidebar as they do normally.

Diff Detail

Repository

rCOMM Comm

Branch

inline-side-bar-ENG-530

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

• benschac created this revision.May 2 2022, 8:25 AM

Herald added subscribers: • karol, • adrian, tomek. · View Herald TranscriptMay 2 2022, 8:25 AM

• benschac mentioned this in D3879: [lib,native,web] move inline sidebar text to shared hook in lib.May 2 2022, 8:26 AM

• benschac retitled this revision from [lib] [refactor] [ENG-530] add memo and nullish check in hook to [lib] [refactor] [ENG-530] add memo and nullish check in hook inline sidebar hook.

Harbormaster completed remote builds in B8713: Diff 12134.May 2 2022, 8:34 AM

• benschac requested review of this revision.May 2 2022, 8:34 AM

ashoat requested changes to this revision.May 2 2022, 8:36 AM

ashoat added inline comments.

lib/hooks/inline-sidebar-text.react.js
17 ↗	(On Diff #12134)	Why are you changing this? If `threadInfo` is typed correctly, there should be no reason for the `?.` operator here This change seems like it makes it possible for `repliesCount` to be 0. I don't think we want that, but I'm not sure If you feel strongly about this change it should probably be in a separate diff.

This revision now requires changes to proceed.May 2 2022, 8:36 AM

• benschac retitled this revision from [lib] [refactor] [ENG-530] add memo and nullish check in hook inline sidebar hook to [lib] [refactor] [ENG-530] add memo and nullish check in hook.May 2 2022, 8:41 AM

• benschac added inline comments.

lib/hooks/inline-sidebar-text.react.js
17 ↗	(On Diff #12134)	I don't feel strongly. That makes sense now that I'm looking at it for a second time.

remove nullish check and optional flag

• benschac retitled this revision from [lib] [refactor] [ENG-530] add memo and nullish check in hook to [lib] [refactor] [ENG-530] add memo in inline sidebar hook .May 2 2022, 8:48 AM

ashoat accepted this revision.May 2 2022, 8:52 AM

This revision is now accepted and ready to land.May 2 2022, 8:52 AM

Harbormaster completed remote builds in B8714: Diff 12135.May 2 2022, 8:53 AM

• benschac added a parent revision: D3879: [lib,native,web] move inline sidebar text to shared hook in lib.May 2 2022, 9:20 AM

Closed by commit rCOMM80abee4eb84a: [lib] [refactor] [ENG-530] add memo in inline sidebar hook (authored by • benschac). · Explain WhyMay 2 2022, 12:58 PM

This revision was automatically updated to reflect the committed changes.

• benschac added a commit: rCOMM80abee4eb84a: [lib] [refactor] [ENG-530] add memo in inline sidebar hook.