Page MenuHomePhabricator

[native] Move methods from Tooltip to functional compononent
ClosedPublic

Authored by angelika on Wed, Dec 11, 7:54 AM.
Tags
None
Referenced Files
Unknown Object (File)
Tue, Dec 24, 1:10 AM
Unknown Object (File)
Mon, Dec 23, 5:24 PM
Unknown Object (File)
Mon, Dec 23, 3:17 PM
Unknown Object (File)
Mon, Dec 23, 9:26 AM
Unknown Object (File)
Sun, Dec 22, 4:16 PM
Unknown Object (File)
Sun, Dec 22, 11:04 AM
Unknown Object (File)
Sun, Dec 22, 1:56 AM
Unknown Object (File)
Sat, Dec 21, 10:06 AM
Subscribers
None

Details

Summary

Move getTooltipItem, onPressMore, renderMoreIcon and onTooltipContainerLayout to the functional compoent as a part of Tooltip migration to functional component.

Depends on D14110

Test Plan

Open up some tooltips and verify they're looking correctly.
Open text message tooltip and press More, verify it works.

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

angelika held this revision as a draft.
angelika edited the test plan for this revision. (Show Details)
ashoat added inline comments.
native/tooltip/tooltip.react.js
562–566 ↗(On Diff #46329)

We don't need to convert this into a callback... and in this case, I don't think we need a React.useMemo either. Arguably we can just pass in tooltipItem={boundTooltipItem}

573–578 ↗(On Diff #46329)

Can be a memo

This revision is now accepted and ready to land.Wed, Dec 11, 5:55 PM
angelika edited the test plan for this revision. (Show Details)

Rebase and address feedback

angelika added inline comments.
native/tooltip/tooltip.react.js
573–578 ↗(On Diff #46329)

I can't put renderMoreIcon in a memo, because BaseTooltipItem expects a callback, not a component

native/tooltip/tooltip.react.js
573–578 ↗(On Diff #46329)

Why can't you change BaseTooltipItem to take a React.Node though?

native/tooltip/tooltip.react.js
573–578 ↗(On Diff #46329)

I didn't realize it's a whole different file... sorry about this, please ignore my feedback here :)