Page MenuHomePhabricator

[native] introduce keyserver selection bottom sheet ui
ClosedPublic

Authored by ginsu on Nov 7 2023, 11:27 AM.
Tags
None
Referenced Files
F3247093: D9754.id33234.diff
Fri, Nov 15, 3:28 AM
F3246943: D9754.id32920.diff
Fri, Nov 15, 2:34 AM
F3246881: D9754.id33229.diff
Fri, Nov 15, 2:16 AM
F3246558: D9754.diff
Fri, Nov 15, 12:38 AM
Unknown Object (File)
Sun, Oct 27, 11:49 PM
Unknown Object (File)
Sun, Oct 27, 11:49 PM
Unknown Object (File)
Sun, Oct 27, 11:49 PM
Unknown Object (File)
Sun, Oct 27, 11:49 PM
Subscribers

Details

Summary

This diff introduces the keyserver selection bottom sheet ui.

Please note that the focus of this diff was to introduce the UI. Subsequent diffs will polish/finish up the keyserver selection bottom sheet remove functionality, fix the snap points so that there is not all this blank space underneath the content of the bottom sheet, handling keyservers that are not connected, etc.

Linear task: https://linear.app/comm/issue/ENG-5469/introduce-keyserverselectionbottomsheet-component-ui

For context here is a screenshot of the designs:

Screenshot 2023-11-07 at 2.35.37 PM.png (1×582 px, 88 KB)

Depends on D9753

Test Plan

Please see the screenshots below

Screenshot 2023-11-07 at 3.01.28 PM.png (1×1 px, 736 KB)

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

ginsu edited the test plan for this revision. (Show Details)
ginsu added reviewers: inka, rohan, michal.
Harbormaster returned this revision to the author for changes because remote builds failed.Nov 7 2023, 11:38 AM
Harbormaster failed remote builds in B23887: Diff 32920!
ginsu edited the test plan for this revision. (Show Details)

rebase

native/profile/keyserver-selection-bottom-sheet.react.js
54 ↗(On Diff #32925)

A subsequent diff will handle this

100 ↗(On Diff #32925)

This color name here is not ideal, but ATM there is no other variable name that gets us this shade of black. I already have a linear task and wrote a comment here that lists out all the names that should be updated/cleaned up

ginsu published this revision for review.Nov 7 2023, 11:21 PM
native/profile/keyserver-selection-bottom-sheet.react.js
100 ↗(On Diff #32925)

Can you fix this now instead of deferring it? We should not be using a drawer color in the keyserver selection screen. The task you linked is extremely generic and the fact that drawerOpenCommunityBackground is listed doesn't really tell me what you're going to do about it or when it's going to be addressed, nor does it make me feel confident that this component in particular will be fixed up

address comments

native/profile/keyserver-selection-bottom-sheet.react.js
54 ↗(On Diff #32983)

A subsequent diff will handle this

native/profile/keyserver-selection-bottom-sheet.react.js
74–75 ↗(On Diff #32983)

According to this the disconnecting is not in the scope for now. I don't think we should be displaying this text until it's implemented

native/profile/keyserver-selection-bottom-sheet.react.js
74–75 ↗(On Diff #32983)

This is a good point, and I was planning on committing to a solution on this today when working on the remove keyserver from keyserver store task. The solution, I've been leaning towards as of now is for is to either have the callback do nothing or show an alert to the user telling them that this functionality is not quite ready yet. I realize that some things are not fully baked yet, but I think it wouldn't hurt to already have this UI implemented, then when the disconnecting functionality is ready we only need to worry about the onPress callback

native/profile/keyserver-selection-bottom-sheet.react.js
74–75 ↗(On Diff #32983)
native/themes/colors.js
103 ↗(On Diff #33143)

Here is what it looks like in light mode (this is also the same as drawerOpenCommunityBackground for light mode)

Screenshot 2023-11-13 at 3.26.38 PM.png (1×1 px, 744 KB)

205 ↗(On Diff #33143)

This newly introduced color is for the accented part of the bottomsheet that shows the keyserver details (name of admin and the url prefix)

Screenshot 2023-11-13 at 3.19.49 PM.png (1×1 px, 738 KB)

The only other variable that uses shadesBlack90 is drawerOpenCommunityBackground. Since we don't want to mix modal and drawer color families, I introduced this new modalAccentBackground

inka added inline comments.
native/profile/keyserver-selection-bottom-sheet.react.js
74–75 ↗(On Diff #32983)

Oh, I see that all of disconnecting is out of scope. This makes sense, thank you

This revision is now accepted and ready to land.Nov 13 2023, 11:56 PM