[lib,native,web] move inline sidebar text to shared hook in lib
ClosedPublic
Actions

Authored by • benschac on Apr 29 2022, 12:39 PM.

Details

Reviewers

atul
tomek
• jacek
• abosh
ashoat

Commits

rCOMMfd5c857f78dc: [lib,native,web] move inline sidebar text to shared hook in lib

Summary

While looking into https://linear.app/comm/issue/ENG-530/inline-sidebar-re-style for web, I noticed that code in inline sidebar is exactly the same in native/ moving this code to a shared hook seemed like the logical choice moving forward with the feature. Additionally, the native design is very close to the web design so I should update them both at the same time to keep consistency.

Test Plan

Everything should work the same as it did before in both web/native. This code controls the text under a message when I new sidebar is created.

Diff Detail

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

• benschac created this revision.Apr 29 2022, 12:39 PM

Herald added subscribers: • karol, • adrian, ashoat. · View Herald TranscriptApr 29 2022, 12:39 PM

• benschac added inline comments.Apr 29 2022, 12:41 PM

lib/hooks/inline-sidebar-text.react.js
32 ↗	(On Diff #12122)	I feel like this should be in a `useMemo`. I'm going to make a new diff since this diff is purely about moving code around.

Harbormaster completed remote builds in B8703: Diff 12122.Apr 29 2022, 12:44 PM

• benschac requested review of this revision.Apr 29 2022, 12:44 PM

• benschac added a child revision: D3878: [web] [remove] [ENG-530] remove arrow from inline reply.May 2 2022, 8:16 AM

• benschac mentioned this in D3887: [lib] [refactor] [ENG-530] add memo in inline sidebar hook .May 2 2022, 8:25 AM

• benschac added inline comments.May 2 2022, 8:26 AM

lib/hooks/inline-sidebar-text.react.js
32 ↗	(On Diff #12122)	https://phabricator.ashoat.com/D3887

Adding @ashoat because @palys-swm is on vacation.

• benschac added a reviewer: ashoat.May 2 2022, 9:21 AM

Thanks for factoring this out! I didn't look too closely at the red/green... assuming that this is just a simple move

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

In D3879#108720, @ashoat wrote:

Thanks for factoring this out! I didn't look too closely at the red/green... assuming that this is just a simple move

Yes, I literally didn't change anything, just moved and deleted.

Closed by commit rCOMMfd5c857f78dc: [lib,native,web] move inline sidebar text to shared hook in lib (authored by • benschac). · Explain WhyMay 2 2022, 12:17 PM

This revision was automatically updated to reflect the committed changes.

• benschac added a commit: rCOMMfd5c857f78dc: [lib,native,web] move inline sidebar text to shared hook in lib.

• benschac mentioned this in rCOMM80abee4eb84a: [lib] [refactor] [ENG-530] add memo in inline sidebar hook.May 2 2022, 12:58 PM