Paths

Table of Contentst

-
lib/
-
selectors/
-
keyserver-selectors.js
-
types/
1/2
keyserver-types.js

[lib] introduce selected keyservers selector
ClosedPublic
Actions

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

Details

Reviewers

inka
michal
rohan

Commits

rCOMM6dfc1c43efbd: [lib] introduce selected keyservers selector

Summary

In order to populate the list of selected keyservers the user has added, we need to introduce this selector which selects all the keyserver infos in the keyserver store and additionally appends the keyserver admin username

Linear task: https://linear.app/comm/issue/ENG-5370/introduce-connected-keyservers-selector

Depends on D9639

Test Plan

Logged out the result of the selector and got the following (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.03.18 AM.png (1×1 px, 170 KB)

Diff Detail

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

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

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

ginsu edited the test plan for this revision. (Show Details)Oct 31 2023, 1:04 AM

ginsu added reviewers: inka, michal, rohan.

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

ginsu edited the summary of this revision. (Show Details)

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

ginsu added a parent revision: D9639: [lib] introduce KeyserverInfos type.

ginsu edited the test plan for this revision. (Show Details)Oct 31 2023, 1:07 AM

ginsu added a child revision: D9642: [native] implement keyserver list ui.Oct 31 2023, 1:19 AM

Harbormaster completed remote builds in B23583: Diff 32518.Oct 31 2023, 1:46 AM

ginsu requested review of this revision.Oct 31 2023, 1:46 AM

Seems reasonable

lib/selectors/keyserver-selectors.js
69–71 ↗	(On Diff #32518)	When do we expect this to be undefined?
lib/types/keyserver-types.js
30 ↗	(On Diff #32518)	Doesn't matter but I think this is a singular `SelectedKeyserverInfo`

This revision is now accepted and ready to land.Oct 31 2023, 6:47 AM

ginsu added inline comments.Oct 31 2023, 12:04 PM

lib/selectors/keyserver-selectors.js
69–71 ↗	(On Diff #32518)	We really should always have this, but the `username` in `UserInfo` is optional https://github.com/CommE2E/comm/blob/master/lib/types/user-types.js#L34-L39 In the `CommunityPill` component we also do something similar where if the keyserver admin username is not defined we return `undefined`, and that is where I got my idea for this implementation https://github.com/CommE2E/comm/blob/master/native/components/community-pill.react.js#L27-L29

rohan added inline comments.Oct 31 2023, 12:06 PM

lib/selectors/keyserver-selectors.js
69–71 ↗	(On Diff #32518)	Ah yeah ok makes sense

address @rohan's comments + rebase before landing

Harbormaster completed remote builds in B23626: Diff 32569.Oct 31 2023, 12:26 PM

update

Harbormaster completed remote builds in B23630: Diff 32574.Oct 31 2023, 9:05 PM

Closed by commit rCOMM6dfc1c43efbd: [lib] introduce selected keyservers selector (authored by ginsu). · Explain WhyOct 31 2023, 9:23 PM

This revision was automatically updated to reflect the committed changes.

ginsu added a commit: rCOMM6dfc1c43efbd: [lib] introduce selected keyservers selector.

ginsu mentioned this in D9726: [native] introduce barebones keyserver selection bottom sheet to root navigator.Nov 9 2023, 8:44 AM

Revision Contents
Changeset List

Path

Size

lib/

selectors/

keyserver-selectors.js

33 lines

types/

keyserver-types.js

5 lines

Diff 32578

View Options

lib/selectors/keyserver-selectors.js

	Show All 21 Lines
	a {			a {
	text-decoration: none;			text-decoration: none;
	color: #2a5db0;			color: #2a5db0;
	}			}
	img,			img,
	iframe {			iframe {
	display: block;			display: block;
	}			}
	input[type='text'],			input[type='text'],
	input[type='password'],			input[type='password'],
	textarea {			textarea {
	atulUnsubmitted Not Done Inline Actions Moving the global input styling and moving it to the input component. Then why are we keeping styles like `border` and `border-radius`? Don't these styles also affect the `<textarea>` tag? Are you sure we want to be removing these styles for that as well? Or should we break this into two separate selectors (one for `input` and another for `textarea`)? atul: > Moving the global input styling and moving it to the input component. Then why are we…
	benschacAuthorUnsubmitted Done Inline Actions Yeah, I should break these styles out a bit more. I don't think they really do anything. But you make a good point that this will also effect `textarea` elements. benschac: Yeah, I should break these styles out a bit more. I don't think they really do anything. But…
	-webkit-appearance: none;
	-moz-appearance: none;
	-webkit-border-radius: 0;
	border: 1px solid #dddddd;			border: 1px solid #dddddd;
	border-radius: 1px;			border-radius: 1px;
	font-family: var(--font-stack);			font-family: var(--font-stack);
	}			}
	:global(#react-root) {			:global(#react-root) {
	display: flex;			display: flex;
	flex-direction: column;			flex-direction: column;
	height: 100%;			height: 100%;
	▲ Show 20 Lines • Show All 243 Lines • Show Last 20 Lines