Page MenuHomePhabricator

[native] Add mappaing from nux tips to button onPress params
ClosedPublic

Authored by inka on Aug 14 2024, 3:51 AM.
Tags
None
Referenced Files
F3527816: D13077.id43483.diff
Tue, Dec 24, 6:54 AM
Unknown Object (File)
Fri, Dec 20, 5:26 AM
Unknown Object (File)
Thu, Dec 19, 10:37 PM
Unknown Object (File)
Sun, Dec 15, 2:44 PM
Unknown Object (File)
Mon, Dec 9, 11:02 AM
Unknown Object (File)
Nov 20 2024, 12:35 PM
Unknown Object (File)
Nov 20 2024, 12:35 PM
Unknown Object (File)
Nov 20 2024, 12:34 PM
Subscribers

Details

Summary

issue: ENG-8617
A handler will display one tip, then pressing on ok will navigate to the next tip and so on. So we need to be able to provide each tip with coordinates and params for the next tip.
We will do this like so: route.params will include a tipKey, and the tip will fetch its coordinates from the context. It will also use this getTipCallbackParams function to get params for the next tip it has to navigate to.

Test Plan

Tested with later diffs - tested that this solution works and it is possible to navigate to next tips

Diff Detail

Repository
rCOMM Comm
Branch
inka/nux
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

inka added inline comments.
native/components/nux-tips-context.react.js
26

This will be changed once a screen for MUTED is added

40–45

We don't need this to be nullable now that the whole object is optional

Harbormaster returned this revision to the author for changes because remote builds failed.Aug 14 2024, 4:04 AM
Harbormaster failed remote builds in B31090: Diff 43376!
native/components/nux-tips-context.react.js
16

Why is it called CallbackParams? Can we find a less implementation-specific name?

Also, we can consider including a tip text in this object.

native/components/nux-tips-context.react.js
16

It doesn't make sense to include text here because we are passing the button to createNUXTipsOverlay anyway, and text directly depends on the button

inka requested review of this revision.Aug 19 2024, 9:04 AM
This revision is now accepted and ready to land.Aug 20 2024, 1:21 AM