Page MenuHomePhabricator

[native] display connnection state of keyserver
ClosedPublic

Authored by ginsu on Oct 31 2023, 1:32 AM.
Tags
None
Referenced Files
F3246151: D9643.diff
Thu, Nov 14, 9:33 PM
Unknown Object (File)
Sun, Nov 10, 11:53 AM
Unknown Object (File)
Wed, Nov 6, 7:27 PM
Unknown Object (File)
Wed, Nov 6, 7:27 PM
Unknown Object (File)
Wed, Nov 6, 7:27 PM
Unknown Object (File)
Wed, Nov 6, 7:26 PM
Unknown Object (File)
Wed, Nov 6, 7:18 PM
Unknown Object (File)
Tue, Oct 29, 2:13 PM

Details

Summary

This diff adds the connection indicator of the keyservers in the keyserver selection list. For context here are the designs

Screenshot 2023-10-31 at 4.13.49 AM.png (1×764 px, 96 KB)

Linear task: https://linear.app/comm/issue/ENG-5369/show-connection-state-of-the-keyserver

Depends on D9642

Test Plan

Please see the screenshots below (the second keyserver is just a test "dummy" keyserver I added to the keyserver store when testing the add keyserver flow)

Screenshot 2023-10-31 at 4.34.07 AM.png (1×1 px, 704 KB)

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

native/themes/colors.js
148–151 ↗(On Diff #32521)

At this moment we did not have any good color variables for these new indicator elements. I think introducing these 4 elements is reasonable and based on the names given here we would be able to reuse these variables for other features

ginsu requested review of this revision.Oct 31 2023, 2:02 AM
rohan added inline comments.
native/profile/keyserver-selection-list.react.js
41–47 ↗(On Diff #32521)

If this works it might be cleaner, but not a big deal so you can keep it if you prefer it the original way

native/themes/colors.js
148–151 ↗(On Diff #32521)

I personally don't see an alternative besides introducing these so they seem ok to me. For sanity it'd be good to make sure they look fine on light mode, but I don't expect any problems there

This revision is now accepted and ready to land.Nov 2 2023, 9:51 AM

address comments + rebase before landing

native/themes/colors.js
148–151 ↗(On Diff #32521)

Confirmed that everything looks good in light mode:

Screenshot 2023-11-02 at 2.59.57 PM.png (1×944 px, 529 KB)