Page MenuHomePhabricator
Differential D13917

[native] make the list of NUX tips to be displayed configurable
ClosedPublic
Actions

Authored by varun on Tue, Nov 12, 12:45 PM.
Tags
None
Referenced Files
F3329417: D13917.id.diff
Wed, Nov 20, 4:56 PM
F3328671: D13917.id45764.diff
Wed, Nov 20, 2:40 PM
F3327041: D13917.id45801.diff
Wed, Nov 20, 8:14 AM
F3324569: D13917.id45799.diff
Wed, Nov 20, 4:39 AM
F3323884: D13917.id45765.diff
Wed, Nov 20, 4:15 AM
Unknown Object (File)
Tue, Nov 19, 11:32 PM
Unknown Object (File)
Tue, Nov 19, 12:58 PM
Unknown Object (File)
Tue, Nov 19, 2:04 AM
View All 15 Files
Subscribers
None

Details

Reviewers
ashoat
tomek
Commits
rCOMMe8675e276b22: [native] make the list of NUX tips to be displayed configurable
Summary

i'll be introducing a new tip that i want to display from the NUX tips overlay, separate from the tips that display on fist launch.

in order to do this, i need to be able to navigate to my new tip directly from the backdrop, as opposed to going through the 4 existing tips first.

in this diff, i remove nextTip from NUXTipParams. instead, in the NUX handler, we initialize an ordered list of tips and use this list to determine the next tip to display

https://linear.app/comm/issue/ENG-9913/make-the-list-of-nux-tips-to-be-displayed-configurable

Test Plan

on a fresh install, confirmed that the same 4 tips appeared as before, and that navigating through them was unchanged

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

varun created this revision.Tue, Nov 12, 12:45 PM
varun edited the summary of this revision. (Show Details)Tue, Nov 12, 1:05 PM
Harbormaster completed remote builds in B32628: Diff 45764.Tue, Nov 12, 1:47 PM
varun requested review of this revision.Tue, Nov 12, 1:47 PM
varun added a child revision: D13920: [native] remove exitingCallback.Tue, Nov 12, 1:51 PM
ashoat accepted this revision.Tue, Nov 12, 2:01 PM
ashoat added 1 blocking reviewer(s): tomek.

Looks good to me! I'm not familiar with the NUX code (it was introduced while I was away) so would appreciate if @tomek could take a look too

native/components/nux-handler.react.js
24–27 ↗(On Diff #45764)

No reason to define this in the component. Can you move it to the top-level scope?

tomek accepted this revision.Wed, Nov 13, 5:18 AM
This revision is now accepted and ready to land.Wed, Nov 13, 5:18 AM
Closed by commit rCOMMe8675e276b22: [native] make the list of NUX tips to be displayed configurable (authored by varun). · Explain WhyWed, Nov 13, 11:00 AM
This revision was automatically updated to reflect the committed changes.
varun added a commit: rCOMMe8675e276b22: [native] make the list of NUX tips to be displayed configurable.

Revision Contents
Changeset List

PathSize
native/
components/
16 lines
5 lines
navigation/
26 lines
tooltip/
31 lines

Diff 45799

View Options

native/components/nux-handler.react.js

Loading...
View Options

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

Loading...
View Options

native/navigation/nux-tip-overlay-backdrop.react.js

Loading...
View Options

native/tooltip/nux-tips-overlay.react.js

Loading...