Paths

Table of Contentst

Differential D5559

[native] Introduce `LoggedOutStaffInfo` and display in `LoggedOutModal`
ClosedPublic
Actions

Authored by atul on Nov 8 2022, 9:53 AM.

Tags

None

Referenced Files

	Unknown Object (File)
	Mon, Nov 25, 1:37 AM

	Unknown Object (File)
	Sun, Nov 24, 6:31 AM

	Unknown Object (File)
	Tue, Nov 19, 8:59 PM

	Unknown Object (File)
	Tue, Nov 19, 8:59 PM

	Unknown Object (File)
	Tue, Nov 19, 8:59 PM

	Unknown Object (File)
	Tue, Nov 19, 8:59 PM

	Unknown Object (File)
	Tue, Nov 19, 8:58 PM

	Unknown Object (File)
	Mon, Nov 4, 3:58 PM

Subscribers

Details

Reviewers

tomek
marcin
varun
ginsu
rohan
ashoat

Group Reviewers

Restricted Owners Package

(Owns No Changed Paths)

Commits

rCOMMf78e5b5d9e81: [native] Introduce `LoggedOutStaffInfo` and display in `LoggedOutModal`

Summary

In order to potentially help with troubleshooting, it's helpful to know

A. What sort of build is running (eg staff release/__DEV__)
B. Whether a staff user has logged in (hasStaffUserLoggedIn)

We display the following fields in LoggedOutStaffInfo:

__DEV__
isStaffRelease
hasStaffUserLoggedIn

Test Plan

Looks as expected:

15bd43.png (1×968 px, 1 MB)

Diff Detail

Repository

Branch

master

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

atul created this revision.Nov 8 2022, 9:53 AM

Herald added subscribers: • abosh, ashoat. · View Herald TranscriptNov 8 2022, 9:53 AM

atul edited the summary of this revision. (Show Details)Nov 8 2022, 9:53 AM

atul edited the test plan for this revision. (Show Details)

atul published this revision for review.Nov 8 2022, 9:56 AM

Harbormaster completed remote builds in B13275: Diff 18217.Nov 8 2022, 10:07 AM

atul added a reviewer: Restricted Owners Package.Nov 9 2022, 7:19 AM

ashoat added inline comments.Nov 9 2022, 8:11 AM

native/account/logged-out-modal.react.js
473	Should these conditions just be moved to `LoggedOutStaffInfo`? That way `LoggedOutModal` (which is already a huge component) doesn't need to track `staffCanSee` or `staffUserHasBeenLoggedIn`. `LoggedOutStaffInfo` can just return `null` in those cases. What do you think?

atul added inline comments.Nov 9 2022, 8:15 AM

native/account/logged-out-modal.react.js
473	I thought it made more sense for the container component to have the logic that determines whether a contained component gets displayed, but can make that change to simplify `LoggedOutModal`

Let's do it... LoggedOutModal is already crazy complicated as-is

This revision now requires changes to proceed.Nov 9 2022, 8:21 AM

address feedback (push down staff hooks to LoggedOutStaffInfo)

native/account/logged-out-modal.react.js
5 ↗	(On Diff #18287)	Shouldn't have imported `useContext` like this to begin with.

cut separate { useContext } import

Harbormaster completed remote builds in B13335: Diff 18287.Nov 9 2022, 11:14 AM

Harbormaster completed remote builds in B13337: Diff 18289.Nov 9 2022, 11:23 AM

atul added 2 blocking reviewer(s): ginsu, rohan.Nov 9 2022, 12:40 PM

ashoat accepted this revision.Nov 9 2022, 6:54 PM

ashoat mentioned this in D5558: [native] Include additional fields in `BuildInfo`.Nov 9 2022, 6:57 PM

atul removed 2 blocking reviewer(s): ginsu, rohan.Nov 9 2022, 7:23 PM

This revision is now accepted and ready to land.Nov 9 2022, 7:23 PM

rebase before landing

This revision was landed with ongoing or failed builds.Nov 9 2022, 7:28 PM

Closed by commit rCOMMf78e5b5d9e81: [native] Introduce `LoggedOutStaffInfo` and display in `LoggedOutModal` (authored by atul). · Explain Why

This revision was automatically updated to reflect the committed changes.

atul added a commit: rCOMMf78e5b5d9e81: [native] Introduce `LoggedOutStaffInfo` and display in `LoggedOutModal`.

Harbormaster completed remote builds in B13349: Diff 18303.Nov 9 2022, 7:41 PM

Revision Contents
Changeset List

Path

Size

native/

account/

logged-out-modal.react.js

13 lines

logged-out-staff-info.react.js

116 lines

Diff 18217

native/account/logged-out-modal.react.js

Loading...

native/account/logged-out-staff-info.react.js

Loading...