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
F2618498: D7576.diff
Fri, Sep 6, 4:55 AM
Unknown Object (File)
Sat, Aug 31, 1:11 PM
Unknown Object (File)
Sun, Aug 18, 7:11 AM
Unknown Object (File)
Thu, Aug 8, 9:32 AM
Unknown Object (File)
Thu, Aug 8, 2:17 AM
Unknown Object (File)
Aug 6 2024, 12:13 PM
Unknown Object (File)
Aug 6 2024, 5:41 AM
Unknown Object (File)
Jul 21 2024, 11:08 AM
View All 79 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
Lint
Lint Not Applicable
Unit
Tests Not Applicable

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
76 lines

Diff 25583

View Options

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

Loading...
View Options

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

Loading...