Page MenuHomePhabricator
Differential D7886

[native] Add wallet identifier tile to ExistingEthereumAccount
ClosedPublic
Actions

Authored by ashoat on May 19 2023, 10:39 AM.
Tags
None
Referenced Files
Unknown Object (File)
Mon, Mar 31, 1:30 AM
Unknown Object (File)
Sun, Mar 23, 2:39 AM
Unknown Object (File)
Wed, Mar 12, 10:01 PM
Unknown Object (File)
Mar 1 2025, 9:37 AM
Unknown Object (File)
Mar 1 2025, 9:37 AM
Unknown Object (File)
Mar 1 2025, 9:35 AM
Unknown Object (File)
Mar 1 2025, 9:23 AM
Unknown Object (File)
Feb 20 2025, 4:35 AM
View All 90 Files
Subscribers
tomek

Details

Reviewers
atul
ginsu
Commits
rCOMM1f14b3067b31: [native] Add wallet identifier tile to ExistingEthereumAccount
Summary

See designs here (for ENS name) and here (for wallet address).

To make it easier to access the wallet address, I also added that to ExistingEthereumAccountParams.

Depends on D7882

Test Plan
ENS nameWallet address
Screenshot 2023-05-19 at 1.35.24 PM.png (2×1 px, 831 KB)
Screenshot 2023-05-19 at 1.35.23 PM.png (2×1 px, 837 KB)

Diff Detail

Repository
rCOMM Comm
Branch
ashoat/registration
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

ashoat created this revision.May 19 2023, 10:39 AM
Herald added a subscriber: tomek. · View Herald TranscriptMay 19 2023, 10:39 AM
ashoat edited the test plan for this revision. (Show Details)May 19 2023, 10:40 AM
ashoat added inline comments.
native/account/registration/existing-ethereum-account.react.js
60

Snuck this copy change in. I think it looks better when it's more concise

ashoat added a child revision: D7887: [native] Update panelSecondaryForeground to shadesBlack80.May 19 2023, 10:57 AM
Harbormaster completed remote builds in B19522: Diff 26689.May 19 2023, 10:57 AM
ashoat requested review of this revision.May 19 2023, 10:57 AM
atul accepted this revision.May 19 2023, 11:29 AM
This revision is now accepted and ready to land.May 19 2023, 11:29 AM
Closed by commit rCOMM1f14b3067b31: [native] Add wallet identifier tile to ExistingEthereumAccount (authored by ashoat). · Explain WhyMay 19 2023, 12:56 PM
This revision was automatically updated to reflect the committed changes.
ashoat added a commit: rCOMM1f14b3067b31: [native] Add wallet identifier tile to ExistingEthereumAccount.

Revision Contents
Changeset List

PathSize
native/
account/
registration/
3 lines
53 lines

Diff 26689

View Options

native/account/registration/connect-ethereum.react.js

Show First 20 Lines • Show All 396 Lines • ▼ Show 20 Lines async ensureAndroidPushNotifsEnabled() {
let [hasPermission] = permissionPromisesResult; let [hasPermission] = permissionPromisesResult;
const [, canRequestPermission] = permissionPromisesResult; const [, canRequestPermission] = permissionPromisesResult;
if (!hasPermission && canRequestPermission) { if (!hasPermission && canRequestPermission) {
const permissionResponse = await (async () => { const permissionResponse = await (async () => {
// We issue a call to sleep to match iOS behavior where prompt // We issue a call to sleep to match iOS behavior where prompt
// doesn't appear immediately but after logged-out modal disappears // doesn't appear immediately but after logged-out modal disappears
await sleep(10); await sleep(10);
await this.requestAndroidNotificationsPermission(); return await this.requestAndroidNotificationsPermission();
marcinUnsubmitted
Not Done
Inline Actions

We definitely should return something from this lambda since otherwise it is useless to assign it to variable.

When we find we don't have Android notif permissions, we ignore the result of requestAndroidNotificationsPermission. It seems like that would cause us to fail to register notif permissions on Android.

If we find that we don't have permissions then just calling await this.requestAndroidNotificationsPermission(); would result in a prompt asking for notifications permissions. However if the user grants those permissions then hasPermissions is still falsy (since promise returned nothing), so deviceToken will be set to null. Nevertheless permissions are actually granted byt the OS, so the state on the device and keyserver would heal itself on next render.

This differential fixes this case so that if user grants permissions correct state is achieved immediately without need for additional re-render to heal the state.

marcin: We definitely should return something from this lambda since otherwise it is useless to assign…
})(); })();
hasPermission = permissionResponse; hasPermission = permissionResponse;
} }
if (!hasPermission) { if (!hasPermission) {
this.failedToRegisterPushPermissionsAndroid(!canRequestPermission); this.failedToRegisterPushPermissionsAndroid(!canRequestPermission);
return; return;
} }
▲ Show 20 Lines • Show All 328 Lines • Show Last 20 Lines
View Options

native/account/registration/existing-ethereum-account.react.js

Show First 20 Lines • Show All 396 Lines • ▼ Show 20 Lines async ensureAndroidPushNotifsEnabled() {
let [hasPermission] = permissionPromisesResult; let [hasPermission] = permissionPromisesResult;
const [, canRequestPermission] = permissionPromisesResult; const [, canRequestPermission] = permissionPromisesResult;
if (!hasPermission && canRequestPermission) { if (!hasPermission && canRequestPermission) {
const permissionResponse = await (async () => { const permissionResponse = await (async () => {
// We issue a call to sleep to match iOS behavior where prompt // We issue a call to sleep to match iOS behavior where prompt
// doesn't appear immediately but after logged-out modal disappears // doesn't appear immediately but after logged-out modal disappears
await sleep(10); await sleep(10);
await this.requestAndroidNotificationsPermission(); return await this.requestAndroidNotificationsPermission();
marcinUnsubmitted
Not Done
Inline Actions

We definitely should return something from this lambda since otherwise it is useless to assign it to variable.

When we find we don't have Android notif permissions, we ignore the result of requestAndroidNotificationsPermission. It seems like that would cause us to fail to register notif permissions on Android.

If we find that we don't have permissions then just calling await this.requestAndroidNotificationsPermission(); would result in a prompt asking for notifications permissions. However if the user grants those permissions then hasPermissions is still falsy (since promise returned nothing), so deviceToken will be set to null. Nevertheless permissions are actually granted byt the OS, so the state on the device and keyserver would heal itself on next render.

This differential fixes this case so that if user grants permissions correct state is achieved immediately without need for additional re-render to heal the state.

marcin: We definitely should return something from this lambda since otherwise it is useless to assign…
})(); })();
hasPermission = permissionResponse; hasPermission = permissionResponse;
} }
if (!hasPermission) { if (!hasPermission) {
this.failedToRegisterPushPermissionsAndroid(!canRequestPermission); this.failedToRegisterPushPermissionsAndroid(!canRequestPermission);
return; return;
} }
▲ Show 20 Lines • Show All 328 Lines • Show Last 20 Lines