Page MenuHomePhabricator
Differential D8702

[web] Add "Use ENS avatar" option to `EditUserAvatarMenu`
ClosedPublic
Actions

Authored by atul on Aug 2 2023, 12:48 AM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Nov 10, 12:32 PM
Unknown Object (File)
Sat, Nov 9, 4:04 AM
Unknown Object (File)
Sun, Nov 3, 5:41 PM
Unknown Object (File)
Fri, Nov 1, 8:20 AM
Unknown Object (File)
Fri, Nov 1, 8:20 AM
Unknown Object (File)
Fri, Nov 1, 8:20 AM
Unknown Object (File)
Fri, Nov 1, 8:20 AM
Unknown Object (File)
Oct 20 2024, 9:02 PM
View All 97 Files
Subscribers
tomek

Details

Reviewers
ashoat
ginsu
Commits
rCOMMe7e6e8a7a1a9: [web] Add "Use ENS avatar" option to `EditUserAvatarMenu`
Summary

Adds menu item to EditUserAvatarMenu to allow SIWE accounts to set ENS avatar.

Test Plan

comm.eth account can set ENS avatar:

atul account cannot set ENS avatar:

2c122c.png (1×1 px, 110 KB)

Diff Detail

Repository
rCOMM Comm
Branch
master
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

atul created this revision.Aug 2 2023, 12:48 AM
Herald added a subscriber: tomek. · View Herald TranscriptAug 2 2023, 12:48 AM
atul published this revision for review.Aug 2 2023, 12:50 AM
atul added inline comments.
web/avatars/edit-user-avatar-menu.react.js
100 ↗(On Diff #29471)

Using the exact copy from native (see D8065)

102 ↗(On Diff #29471)

Shoutout @tomek for adding iconComponent prop to MenuItem in D8306 which made things more flexible.

atul edited the test plan for this revision. (Show Details)Aug 2 2023, 12:51 AM
Harbormaster completed remote builds in B21481: Diff 29471.Aug 2 2023, 1:07 AM
ginsu accepted this revision.Aug 2 2023, 9:03 AM

accepting but please address comment inline before landing

web/avatars/edit-user-avatar-menu.react.js
120–125 ↗(On Diff #29471)

I would personally prefer if instead of this ternary expression, we implemented menuItems by pushing the menu items into a "results" array based off condition like we did in native: https://github.com/CommE2E/comm/blob/master/native/avatars/edit-user-avatar.react.js#L89C3-L113C6

This revision is now accepted and ready to land.Aug 2 2023, 9:03 AM
atul updated this revision to Diff 29496.Aug 2 2023, 7:11 PM

address feedback

atul added inline comments.Aug 2 2023, 7:19 PM
web/avatars/edit-user-avatar-menu.react.js
125–127

Personally would strongly prefer if "Reset to default" was always visible (grayed out if anything), but I'll match native for consistency.

Harbormaster failed remote builds in B21501: Diff 29496!Aug 2 2023, 7:38 PM
atul updated this revision to Diff 29497.Aug 2 2023, 8:56 PM

rebase and land

This revision was landed with ongoing or failed builds.Aug 2 2023, 9:01 PM
Closed by commit rCOMMe7e6e8a7a1a9: [web] Add "Use ENS avatar" option to `EditUserAvatarMenu` (authored by atul). · Explain Why
This revision was automatically updated to reflect the committed changes.
atul added a commit: rCOMMe7e6e8a7a1a9: [web] Add "Use ENS avatar" option to `EditUserAvatarMenu`.
Harbormaster failed remote builds in B21502: Diff 29497!Aug 2 2023, 9:23 PM

Revision Contents
Changeset List

PathSize
web/
avatars/
54 lines
components/
1 line

Diff 29496

View Options

web/avatars/edit-user-avatar-menu.react.js

Loading...
View Options

web/components/menu.css

Loading...