Differential D13079

[native] Fetch coordinates from context in nux tips overlay
ClosedPublic
Actions

Authored by inka on Aug 14 2024, 5:34 AM.

Tags

None

Referenced Files

	F3204739: D13079.id43464.diff
	Sat, Nov 9, 9:50 PM

	F3204062: D13079.diff
	Sat, Nov 9, 8:46 PM

	F3193597: D13079.id43464.diff
	Fri, Nov 8, 11:06 PM

	Unknown Object (File)
	Fri, Nov 8, 6:34 PM

	Unknown Object (File)
	Fri, Nov 8, 6:15 PM

	Unknown Object (File)
	Fri, Nov 8, 12:23 PM

	Unknown Object (File)
	Fri, Oct 18, 9:48 PM

	Unknown Object (File)
	Fri, Oct 18, 9:42 PM

View All 55 Files

Subscribers

Details

Reviewers

tomek
will

Commits

rCOMM3a3c6753b4ad: [native] Fetch coordinates from context in nux tips overlay

Summary

issue: ENG-8617
As discussed in previous diff, we want the tip to fetch its coordinates from the context, instead of getting them in the route params

Test Plan

tested with following diffs, tested that the tip can be displayed and it fetches its coordinates correctly

Diff Detail

Repository

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

inka created this revision.Aug 14 2024, 5:34 AM

Herald added a subscriber: ashoat. · View Herald TranscriptAug 14 2024, 5:34 AM

inka added a parent revision: D13077: [native] Add mappaing from nux tips to button onPress params.Aug 14 2024, 5:35 AM

inka added a child revision: D13080: [native] Add <next> button to nux tips.Aug 14 2024, 5:41 AM

Harbormaster completed remote builds in B31094: Diff 43380.Aug 14 2024, 5:51 AM

inka requested review of this revision.Aug 14 2024, 5:51 AM

tomek requested changes to this revision.Aug 14 2024, 9:22 AM

tomek added inline comments.

native/tooltip/nux-tips-overlay.react.js
15–16 ↗	(On Diff #43380)	Can be merged.
145–147 ↗	(On Diff #43380)	It is a strange pattern to create a callback and immediately invoke it on every render. Why do we need to do it? It looks like this approach can be replaced by a simple memo.

This revision now requires changes to proceed.Aug 14 2024, 9:22 AM

Address review

Harbormaster completed remote builds in B31150: Diff 43464.Aug 19 2024, 9:10 AM

inka added a child revision: D13118: [native] Rename RegistrationButton.Aug 20 2024, 12:33 AM

inka removed a child revision: D13080: [native] Add <next> button to nux tips.

tomek accepted this revision.Aug 20 2024, 7:31 AM

tomek added inline comments.

native/tooltip/nux-tips-overlay.react.js
125 ↗	(On Diff #43464)	This line was invalid, because it wasn't guaranteed for a render to happen, but after changes from D13085 it is safe.

This revision is now accepted and ready to land.Aug 20 2024, 7:31 AM

Closed by commit rCOMM3a3c6753b4ad: [native] Fetch coordinates from context in nux tips overlay (authored by inka). · Explain WhyAug 21 2024, 12:07 AM

This revision was automatically updated to reflect the committed changes.

inka added a commit: rCOMM3a3c6753b4ad: [native] Fetch coordinates from context in nux tips overlay.

Revision Contents
Changeset List

Path

Size

native/

tooltip/

nux-tips-overlay.react.js

38 lines

Diff 43529

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

Loading...