Page MenuHomePhabricator

[native] Move onTipContainerLayout
ClosedPublic

Authored by inka on Aug 5 2024, 6:05 AM.
Tags
None
Referenced Files
F2759792: D12980.id43344.diff
Thu, Sep 19, 5:43 AM
F2759767: D12980.id43344.diff
Thu, Sep 19, 5:28 AM
Unknown Object (File)
Thu, Sep 5, 3:52 PM
Unknown Object (File)
Thu, Sep 5, 3:52 PM
Unknown Object (File)
Thu, Sep 5, 3:51 PM
Unknown Object (File)
Wed, Sep 4, 2:56 AM
Unknown Object (File)
Sun, Sep 1, 3:40 PM
Unknown Object (File)
Sat, Aug 31, 6:49 PM
Subscribers

Details

Summary

issue: ENG-8914
Refactoring from class to function

Test Plan

Tested by creating a nux tooltip and displaying it - still works

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

inka requested review of this revision.Aug 5 2024, 8:21 AM
native/tooltip/nux-tips-overlay.react.js
300 ↗(On Diff #43115)

It's a bit hacky to use a useMemo for this. I think the usual "best practice" is that an instance variable from a class component becomes a useRef in a function component. This is functionally the same in our configuration of React, but it might become more important in "concurrent mode", if we ever enable that (might be related to React Native "new architecture")

tomek added inline comments.
native/tooltip/nux-tips-overlay.react.js
300 ↗(On Diff #43115)

Another reason for using ref here is that we usually assume that values returned by memos are immutable. The value that we have here can mutate without its reference being changed, which is confusing.

319 ↗(On Diff #43115)

We should make the dependencies more specific.

This revision is now accepted and ready to land.Aug 6 2024, 2:25 AM
This revision was automatically updated to reflect the committed changes.