[native] display connnection state of keyserver
ClosedPublic
Actions

Authored by ginsu on Oct 31 2023, 1:32 AM.

Details

Reviewers

inka
rohan
michal

Commits

rCOMM4082f259b587: [native] display connnection state of keyserver

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

No Lint Coverage

Unit

No Test Coverage

Event Timeline

ginsu created this revision.Oct 31 2023, 1:32 AM

Herald added subscribers: will, • ted, tomek, ashoat. · View Herald TranscriptOct 31 2023, 1:32 AM

ginsu edited the summary of this revision. (Show Details)Oct 31 2023, 1:35 AM

ginsu edited the test plan for this revision. (Show Details)

ginsu added reviewers: inka, rohan, michal.

ginsu added a parent revision: D9642: [native] implement keyserver list ui.

ginsu attached a referenced file: F843836: Screenshot 2023-10-31 at 4.34.07 AM.png. (Show Details)

ginsu attached a referenced file: F843835: Screenshot 2023-10-31 at 4.13.49 AM.png. (Show Details)

ginsu added inline comments.Oct 31 2023, 1:38 AM

native/themes/colors.js
148–151	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

Harbormaster completed remote builds in B23586: Diff 32521.Oct 31 2023, 2:02 AM

ginsu requested review of this revision.Oct 31 2023, 2:02 AM

rohan accepted this revision.Nov 2 2023, 9:51 AM

rohan added inline comments.

native/profile/keyserver-selection-list.react.js
41–47	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	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