Page MenuHomePhabricator
Differential D7576

[native] Introduce RegistrationTile and fill in KeyserverSelection
ClosedPublic
Actions

Authored by ashoat on Apr 22 2023, 12:48 PM.
Tags
None
Referenced Files
Unknown Object (File)
Oct 3 2024, 4:46 PM
Unknown Object (File)
Oct 3 2024, 5:41 AM
Unknown Object (File)
Sep 26 2024, 12:29 PM
Unknown Object (File)
Sep 18 2024, 4:26 PM
Unknown Object (File)
Sep 18 2024, 4:22 PM
Unknown Object (File)
Sep 18 2024, 4:18 PM
Unknown Object (File)
Sep 18 2024, 4:16 PM
Unknown Object (File)
Sep 18 2024, 4:04 PM
View All 96 Files
Subscribers
tomek

Details

Reviewers
ginsu
atul
Commits
rCOMMb5a2f08b7c5b: [native] Introduce RegistrationTile and fill in KeyserverSelection
Summary

This diff styles KeyserverSelection to match what's in Figma, but without any selection state or "Next" button.

I also introduce two new components: KeyserverTile and KeyserverTileHeader. They serve to factor out the tile JSX, and in later diffs will also hold the selection state.

Depends on D7575

Test Plan
beforeafter
Screenshot 2023-04-22 at 3.28.38 PM.png (1×1 px, 704 KB)
Screenshot 2023-04-22 at 3.28.44 PM.png (1×1 px, 785 KB)

Diff Detail

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

Event Timeline

ashoat created this revision.Apr 22 2023, 12:48 PM
Herald added a subscriber: tomek. · View Herald TranscriptApr 22 2023, 12:48 PM
ashoat edited the test plan for this revision. (Show Details)Apr 22 2023, 12:50 PM
ashoat attached a referenced file: F502096: Screenshot 2023-04-22 at 3.28.38 PM.png. (Show Details)
ashoat added inline comments.Apr 22 2023, 12:53 PM
native/account/registration/registration-tile.react.js
16–25 ↗(On Diff #25566)

This looks kinda hacky, but I personally think it's a good pattern for making a React component that needs two React.Node props (as opposed to the usual single children prop) look "semantic".

<RegistrationTile>
  <RegistrationTileHeader>
    {headerGoesHere}
  </RegistrationTileHeader>
  {bodyGoesHere}
</RegistrationTile>

Open to making this less "magical" if people want. One alternative would be for RegistrationTile to take a "render prop" that renders the header.

ashoat edited the summary of this revision. (Show Details)Apr 22 2023, 12:54 PM
Harbormaster returned this revision to the author for changes because remote builds failed.Apr 22 2023, 1:00 PM
Harbormaster failed remote builds in B18666: Diff 25566!
ashoat updated this revision to Diff 25567.Apr 22 2023, 1:28 PM
  1. Make it work with Fast Refresh by comparing child.type.name instead of child.type
  2. Memoize the header and body
ashoat updated this revision to Diff 25568.Apr 22 2023, 1:34 PM

Move radio rendering from RegistrationTile to RegistrationTileHeader

Harbormaster failed remote builds in B18667: Diff 25567!Apr 22 2023, 1:42 PM
ashoat added a child revision: D7577: [native] Add selection state to KeyserverSelection.Apr 22 2023, 1:42 PM
Harbormaster returned this revision to the author for changes because remote builds failed.Apr 22 2023, 1:46 PM
Harbormaster failed remote builds in B18668: Diff 25568!
ashoat updated this revision to Diff 25573.Apr 22 2023, 2:35 PM

Rebase on Android CI fix

Harbormaster completed remote builds in B18673: Diff 25573.Apr 22 2023, 2:52 PM
ashoat requested review of this revision.Apr 22 2023, 2:52 PM
ashoat added inline comments.Apr 22 2023, 3:01 PM
native/account/registration/registration-tile.react.js
14–28 ↗(On Diff #25573)

This looks kinda hacky, but I personally think it's a good pattern for making a React component that needs two React.Node props (as opposed to the usual single children prop) look "semantic".

<RegistrationTile>
  <RegistrationTileHeader>
    {headerGoesHere}
  </RegistrationTileHeader>
  {bodyGoesHere}
</RegistrationTile>

Open to making this less "magical" if people want. One alternative would be for RegistrationTile to take a "render prop" that renders the header.

atul accepted this revision.Apr 23 2023, 11:49 AM
This revision is now accepted and ready to land.Apr 23 2023, 11:49 AM
Closed by commit rCOMMb5a2f08b7c5b: [native] Introduce RegistrationTile and fill in KeyserverSelection (authored by ashoat). · Explain WhyApr 23 2023, 4:40 PM
This revision was automatically updated to reflect the committed changes.
ashoat added a commit: rCOMMb5a2f08b7c5b: [native] Introduce RegistrationTile and fill in KeyserverSelection.

Revision Contents
Changeset List

PathSize
native/
account/
registration/
75 lines
71 lines

Diff 25567

View Options

native/account/registration/keyserver-selection.react.js

Loading...
View Options

native/account/registration/registration-tile.react.js

Loading...