Paths

Table of Contentst

-
services/backup/blob_client/src/
-
backup/
-
blob_client/
-
src/
2/5
get_client.rs
2/5
lib.rs
2/5
put_client.rs

[services] Rust Integration - Backup - Rust - Multiple put clients
ClosedPublic
Actions

Authored by • karol on Sep 2 2022, 3:52 AM.

Details

Reviewers

• jon
tomek
varun
• max

Commits

rCOMM1a07b32b94a7: [services] Rust Integration - Backup - Rust - Multiple put clients

Summary

Depends on D5034

In order to be able to handle multiple connections at the same time, we have to store a collection of clients in the rust state instead of having just one.

Here, I'm adding the collection to the put blob client.

Test Plan

The backup service will not build successfully because of invalid calls from the c++ but I decided to divide the code so the diffs are not too big. This is fixed in the next diff in the stack - D5036

This will work:

cd services/backup/blob_client
cargo check

Diff Detail

Repository

rCOMM Comm

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

• karol created this revision.Sep 2 2022, 3:52 AM

Herald added a reviewer: • jon. · View Herald TranscriptSep 2 2022, 3:52 AM

Herald added subscribers: • abosh, atul, tomek, ashoat. · View Herald Transcript

Harbormaster returned this revision to the author for changes because remote builds failed.Sep 2 2022, 3:55 AM

Harbormaster failed remote builds in B11829: Diff 16234!

Backup doesn't build but this is explained in the test plan section.

Overall this might work but it seems like having a pool of clients and a queue with tasks could be a better solution.

services/backup/blob_client/src/put_client.rs
41	This probably could be made more efficient by using `RwLock`
102–108	We should format this code in the diff that introduced it.
305	Can we make this message more verbose instead of adding just numbers?

This revision now requires changes to proceed.Sep 5 2022, 7:58 AM

• karol added inline comments.Sep 6 2022, 7:03 AM

services/backup/blob_client/src/put_client.rs
41	I don't know, we probably should discuss that, mutex seems to be the easiest solution. I think this needs more research/discussion. https://linear.app/comm/issue/ENG-1741/consider-using-rwlock-instead-of-mutex
102–108	you're right
305	sure

address comments

Harbormaster failed remote builds in B11921: Diff 16367!Sep 6 2022, 7:13 AM

I don't think an approach where every function takes the same parameter is a maintainable one. If all the functions share the same context, we should probably make this context a field of a struct and implement all the functions as methods of this struct. Is it possible to do something like that in cxx?

services/backup/blob_client/src/put_client.rs
49–54 ↗	(On Diff #16367)	Can we keep the approach with `match`?
140–143 ↗	(On Diff #16367)	Please avoid formatting the code that was introduced earlier

This revision now requires changes to proceed.Sep 7 2022, 6:10 AM

• karol added inline comments.Sep 8 2022, 6:25 AM

services/backup/blob_client/src/put_client.rs
49–54 ↗	(On Diff #16367)	I don't mind
140–143 ↗	(On Diff #16367)	I'll try. As this code's landed already, I gues I'm going to have to let this one go.

address comments

Harbormaster failed remote builds in B12014: Diff 16496!Sep 8 2022, 6:30 AM

tomek requested changes to this revision.Sep 9 2022, 4:34 AM

tomek added inline comments.

services/backup/blob_client/src/lib.rs
18–20 ↗	(On Diff #16496)	Is there a good reason for using `*const c_char`? Are there any other CXX types that represent a string and don't require using raw pointers?
services/backup/blob_client/src/put_client.rs
47 ↗	(On Diff #16496)	Is there a downside of using `&str`?

This revision now requires changes to proceed.Sep 9 2022, 4:34 AM

• karol added inline comments.Sep 9 2022, 5:13 AM

services/backup/blob_client/src/lib.rs
18–20 ↗	(On Diff #16496)	In all tutorials they use `const char*` but you made me think and I don't see why `rust::String` couldn't work. It is derived in the cxx bridge header file, so it is accessible and can be constructed from the c++ `std::string`. I've not tried it though. I'm going to give it a try, thanks.
services/backup/blob_client/src/put_client.rs
47 ↗	(On Diff #16496)	And is there one of using `&String`? I think it's better to borrow a heap-allocated string instead of converting it into a `str`, right?

• karol mentioned this in D5036: [services] Rust Integration - Backup - c++ - Multiple put clients.Sep 9 2022, 6:44 AM

• karol requested review of this revision.Sep 9 2022, 7:11 AM

• karol mentioned this in D5099: [services] Backup - Blob Get Client - Pass rust string instead of const char*.

• karol added inline comments.

services/backup/blob_client/src/lib.rs
18–20 ↗	(On Diff #16496)	D5099

• jon added inline comments.Sep 12 2022, 9:18 AM

services/backup/blob_client/src/put_client.rs
47 ↗	(On Diff #16496)	Could also use `rust::Str` https://cxx.rs/binding/str.html, and then this would be `&str` on the rust side which is more appropriate.

The CI reported some compilation issues

services/backup/blob_client/src/lib.rs
18–20 ↗	(On Diff #16496)	It would be a lot better to update this diff instead of creating a followup
services/backup/blob_client/src/put_client.rs
40 ↗	(On Diff #16496)	Can we update this diff to not use `Arc`?
47 ↗	(On Diff #16496)	And is there one of using `&String`? Yet, it is https://blog.logrocket.com/understanding-rust-string-str/ For example, since &String can be coerced to &str, but not the other way around, it usually makes sense to use &str as a parameter type, if you just need a read-only view of a string. So using `&str` in every place where it can be used is a good practice because it makes the API easier to use. On the other hand Further, if a function needs to mutate a given String, there is no point in passing in a &str, since it will be immutable and you would have to create a String from it and return it afterwards. If mutation is what you need, &mut String is the way to go. So if the function mutates, then `String` is a good idea. But in all the cases `&str` is strongly preferred, unless it can't be used.

This revision now requires changes to proceed.Sep 13 2022, 7:16 AM

• jon added inline comments.Sep 13 2022, 12:16 PM

services/backup/blob_client/src/put_client.rs
40 ↗	(On Diff #16496)	https://phab.comm.dev/D5114 was merged, should just need to rebase now

• karol added inline comments.Sep 14 2022, 1:19 AM

services/backup/blob_client/src/put_client.rs
47 ↗	(On Diff #16496)	Ok, fair enough, will change to `&str` then.

use &str

Harbormaster failed remote builds in B12130: Diff 16652!Sep 14 2022, 1:23 AM

would really like for this to be rebased, the Arc usage for the other lazy_static items has been removed on master, and would like to avoid potentially adding them back.

This revision now requires changes to proceed.Sep 14 2022, 10:48 AM

rebase

Harbormaster failed remote builds in B12157: Diff 16685!Sep 15 2022, 1:30 AM

Btw, if that was the only concern I think you could just accept it, rebasing to master was pretty easy and straightforward and would enable landing some diffs already reducing cycles.

tomek accepted this revision.Sep 15 2022, 7:15 AM

• jon accepted this revision.Sep 15 2022, 4:23 PM

This revision is now accepted and ready to land.Sep 15 2022, 4:23 PM

rebase

This revision was landed with ongoing or failed builds.Sep 16 2022, 1:54 AM

Closed by commit rCOMM1a07b32b94a7: [services] Rust Integration - Backup - Rust - Multiple put clients (authored by • karol). · Explain Why

This revision was automatically updated to reflect the committed changes.

• karol added a commit: rCOMM1a07b32b94a7: [services] Rust Integration - Backup - Rust - Multiple put clients.

Harbormaster failed remote builds in B12201: Diff 16736!Sep 16 2022, 1:56 AM

Not sure what happened here, but according to CI

This revision was landed with ongoing or failed builds.

We should (almost) never land anything with failed builds!

• karol mentioned this in rCOMMfad989a99d6c: [services] Backup - Blob Get Client - Pass rust string instead of const char*.Sep 19 2022, 7:28 AM

Revision Contents
Changeset List

Path

Size

services/

backup/

blob_client/

src/

get_client.rs

2 lines

lib.rs

13 lines

put_client.rs

109 lines

Diff 16234

View Options

services/backup/blob_client/src/get_client.rs

Show First 20 Lines • Show All 42 Lines • ▼ Show 20 Lines
import {		import {
RelationshipListItemTooltipModalRouteName,		RelationshipListItemTooltipModalRouteName,
FriendListRouteName,		FriendListRouteName,
BlockListRouteName,		BlockListRouteName,
} from '../navigation/route-names.js';		} from '../navigation/route-names.js';
import { useSelector } from '../redux/redux-utils.js';		import { useSelector } from '../redux/redux-utils.js';
import { type Colors, useColors, useStyles } from '../themes/colors.js';		import { type Colors, useColors, useStyles } from '../themes/colors.js';
import type { VerticalBounds } from '../types/layout-types.js';		import type { VerticalBounds } from '../types/layout-types.js';
		import { useNavigateToUserProfileBottomSheet } from '../user-profile/user-profile-utils.js';
import Alert from '../utils/alert.js';		import Alert from '../utils/alert.js';

type BaseProps = {		type BaseProps = {
+userInfo: AccountUserInfo,		+userInfo: AccountUserInfo,
+lastListItem: boolean,		+lastListItem: boolean,
+verticalBounds: ?VerticalBounds,		+verticalBounds: ?VerticalBounds,
+relationshipListRoute: NavigationRoute<'FriendList' \| 'BlockList'>,		+relationshipListRoute: NavigationRoute<'FriendList' \| 'BlockList'>,
+navigate: RelationshipListNavigate,		+navigate: RelationshipListNavigate,
Show All 10 Lines	type Props = {
// async functions that hit server APIs		// async functions that hit server APIs
+updateRelationships: (		+updateRelationships: (
request: RelationshipRequest,		request: RelationshipRequest,
) => Promise<RelationshipErrors>,		) => Promise<RelationshipErrors>,
// withOverlayContext		// withOverlayContext
+overlayContext: ?OverlayContextType,		+overlayContext: ?OverlayContextType,
// withKeyboardState		// withKeyboardState
+keyboardState: ?KeyboardState,		+keyboardState: ?KeyboardState,
		+navigateToUserProfileBottomSheet: (userID: string) => mixed,
};		};
class RelationshipListItem extends React.PureComponent<Props> {		class RelationshipListItem extends React.PureComponent<Props> {
editButton = React.createRef<React.ElementRef<typeof View>>();		editButton = React.createRef<React.ElementRef<typeof View>>();

render() {		render() {
const {		const {
lastListItem,		lastListItem,
removeUserLoadingStatus,		removeUserLoadingStatus,
▲ Show 20 Lines • Show All 77 Lines • ▼ Show 20 Lines	) {
>		>
<Text style={this.props.styles.blueAction}>Add</Text>		<Text style={this.props.styles.blueAction}>Add</Text>
</TouchableOpacity>		</TouchableOpacity>
);		);
}		}

return (		return (
<View style={[this.props.styles.container, borderBottom]}>		<View style={[this.props.styles.container, borderBottom]}>
		<TouchableOpacity
		ginsuAuthorUnsubmitted Done Inline Actions The red background shows the `TouchableOpacity`: I think it would be a be a better user experience if we had the entire item be a touchable instead of just the avatar/username ginsu: The red background shows the `TouchableOpacity`: {F773403} I think it would be a be a better…
		atulUnsubmitted Not Done Inline Actions What surfaces should be "touchable" is probably something we should consider as part of the Design System (at some point) CC @ashoat @ted In this particular case, it feels a little weird that we're extending so far to the right, but on the left we go just until the left edge. I guess this is a new scenario, because previously we were just listing "static" avatars/usernames and left all the interaction to the right-justified icon. Unsolicited, but if it were up to me I think it'd make sense to give the avatar/username some sort of inset to make it clear that A. the element is interactive B. the bounds of the tap target Something like: or atul: What surfaces should be "touchable" is probably something we should consider as part of the…
		ashoatUnsubmitted Not Done Inline Actions I like @atul's idea. Curious for @ted's thoughts, both on Atul's idea and also on how we can include touchable surface / interaction design of touchables in the design system ashoat: I like @atul's idea. Curious for @ted's thoughts, both on Atul's idea and also on how we can…
		tedUnsubmitted Not Done Inline Actions In this particular case, it feels a little weird that we're extending so far to the right, but on the left we go just until the left edge. I think the touch point can extend more on the left, to where `Search` starts at the input field above. @atul @ashoat I think adding touch points to the design system makes perfect sense and can be included for both web and mobile components. Each design system component lives on its own page in Figma, which will also include small documents to specifically point out the interactive touch points. These documents can point out other component rules such as places to use this component, certain rules of how to use this component, etc. For the concepts you shared, I don't think that adding an inset is necessary to point out that the element is interactive. Adding an inset feels a bit cluttered to the overall list layout. If we do want to use an inset container for the avatar and username, then reducing the separator lines (smaller, more subtle) will help with over-cluttering. I took a quick look at Discord's username touchpoints in their list. In the video, I am tapping as far right as I can before the two action buttons. The entire width seems tappable except for the two buttons. Not saying we need to do exactly how Discord handles this interaction, I wanted to see how other apps were handling this. With that, I think the UI is fine how it is, but always open if more people think we should add an inset or interactive indicator. Thank you! ted: > In this particular case, it feels a little weird that we're extending so far to the right…
		ginsuAuthorUnsubmitted Done Inline Actions For the concepts you shared, I don't think that adding an inset is necessary to point out that the element is interactive. Adding an inset feels a bit cluttered to the overall list layout. If we do want to use an inset container for the avatar and username, then reducing the separator lines (smaller, more subtle) will help with over-cluttering. I agree with this point, I think especially since we have the edit (pencil) icon to the right it would feel strange if that didn't have an inset/obvious indicator that the icon is touchable but the avatar/username does. To polish this up, I can make the entire container a touchable or at the very least, also extend the touchable to be touching the left side of this container ginsu: > For the concepts you shared, I don't think that adding an inset is necessary to point out…
		onPress={this.onPressUser}
		style={this.props.styles.userInfoContainer}
		>
<UserAvatar size="small" userID={this.props.userInfo.id} />		<UserAvatar size="small" userID={this.props.userInfo.id} />
<SingleLine style={this.props.styles.username}>		<SingleLine style={this.props.styles.username}>
{this.props.userInfo.username}		{this.props.userInfo.username}
</SingleLine>		</SingleLine>
		</TouchableOpacity>
{editButton}		{editButton}
</View>		</View>
);		);
}		}

		onPressUser = () => {
		this.props.navigateToUserProfileBottomSheet(this.props.userInfo.id);
		};

onSelect = () => {		onSelect = () => {
const { id, username } = this.props.userInfo;		const { id, username } = this.props.userInfo;
this.props.onSelect({ id, username });		this.props.onSelect({ id, username });
};		};

visibleEntryIDs() {		visibleEntryIDs() {
const { relationshipListRoute } = this.props;		const { relationshipListRoute } = this.props;
const id = {		const id = {
▲ Show 20 Lines • Show All 85 Lines • ▼ Show 20 Lines	container: {
paddingHorizontal: 24,		paddingHorizontal: 24,
paddingVertical: 10,		paddingVertical: 10,
backgroundColor: 'panelForeground',		backgroundColor: 'panelForeground',
borderColor: 'panelForegroundBorder',		borderColor: 'panelForegroundBorder',
},		},
borderBottom: {		borderBottom: {
borderBottomWidth: 1,		borderBottomWidth: 1,
},		},
		userInfoContainer: {
		flex: 1,
		flexDirection: 'row',
		},
buttonContainer: {		buttonContainer: {
flexDirection: 'row',		flexDirection: 'row',
},		},
editButtonWithMargin: {		editButtonWithMargin: {
marginLeft: 15,		marginLeft: 15,
},		},
username: {		username: {
color: 'panelForegroundSecondaryLabel',		color: 'panelForegroundSecondaryLabel',
Show All 29 Lines	) {
);		);
const colors = useColors();		const colors = useColors();
const styles = useStyles(unboundStyles);		const styles = useStyles(unboundStyles);
const dispatchActionPromise = useDispatchActionPromise();		const dispatchActionPromise = useDispatchActionPromise();
const boundUpdateRelationships = useServerCall(updateRelationships);		const boundUpdateRelationships = useServerCall(updateRelationships);
const overlayContext = React.useContext(OverlayContext);		const overlayContext = React.useContext(OverlayContext);
const keyboardState = React.useContext(KeyboardContext);		const keyboardState = React.useContext(KeyboardContext);

		const navigateToUserProfileBottomSheet =
		useNavigateToUserProfileBottomSheet();

return (		return (
<RelationshipListItem		<RelationshipListItem
{...props}		{...props}
removeUserLoadingStatus={removeUserLoadingStatus}		removeUserLoadingStatus={removeUserLoadingStatus}
colors={colors}		colors={colors}
styles={styles}		styles={styles}
dispatchActionPromise={dispatchActionPromise}		dispatchActionPromise={dispatchActionPromise}
updateRelationships={boundUpdateRelationships}		updateRelationships={boundUpdateRelationships}
overlayContext={overlayContext}		overlayContext={overlayContext}
keyboardState={keyboardState}		keyboardState={keyboardState}
		navigateToUserProfileBottomSheet={navigateToUserProfileBottomSheet}
/>		/>
);		);
});		});

export default ConnectedRelationshipListItem;		export default ConnectedRelationshipListItem;

View Options

services/backup/blob_client/src/lib.rs

Show First 20 Lines • Show All 42 Lines • ▼ Show 20 Lines
import {		import {
RelationshipListItemTooltipModalRouteName,		RelationshipListItemTooltipModalRouteName,
FriendListRouteName,		FriendListRouteName,
BlockListRouteName,		BlockListRouteName,
} from '../navigation/route-names.js';		} from '../navigation/route-names.js';
import { useSelector } from '../redux/redux-utils.js';		import { useSelector } from '../redux/redux-utils.js';
import { type Colors, useColors, useStyles } from '../themes/colors.js';		import { type Colors, useColors, useStyles } from '../themes/colors.js';
import type { VerticalBounds } from '../types/layout-types.js';		import type { VerticalBounds } from '../types/layout-types.js';
		import { useNavigateToUserProfileBottomSheet } from '../user-profile/user-profile-utils.js';
import Alert from '../utils/alert.js';		import Alert from '../utils/alert.js';

type BaseProps = {		type BaseProps = {
+userInfo: AccountUserInfo,		+userInfo: AccountUserInfo,
+lastListItem: boolean,		+lastListItem: boolean,
+verticalBounds: ?VerticalBounds,		+verticalBounds: ?VerticalBounds,
+relationshipListRoute: NavigationRoute<'FriendList' \| 'BlockList'>,		+relationshipListRoute: NavigationRoute<'FriendList' \| 'BlockList'>,
+navigate: RelationshipListNavigate,		+navigate: RelationshipListNavigate,
Show All 10 Lines	type Props = {
// async functions that hit server APIs		// async functions that hit server APIs
+updateRelationships: (		+updateRelationships: (
request: RelationshipRequest,		request: RelationshipRequest,
) => Promise<RelationshipErrors>,		) => Promise<RelationshipErrors>,
// withOverlayContext		// withOverlayContext
+overlayContext: ?OverlayContextType,		+overlayContext: ?OverlayContextType,
// withKeyboardState		// withKeyboardState
+keyboardState: ?KeyboardState,		+keyboardState: ?KeyboardState,
		+navigateToUserProfileBottomSheet: (userID: string) => mixed,
};		};
class RelationshipListItem extends React.PureComponent<Props> {		class RelationshipListItem extends React.PureComponent<Props> {
editButton = React.createRef<React.ElementRef<typeof View>>();		editButton = React.createRef<React.ElementRef<typeof View>>();

render() {		render() {
const {		const {
lastListItem,		lastListItem,
removeUserLoadingStatus,		removeUserLoadingStatus,
▲ Show 20 Lines • Show All 77 Lines • ▼ Show 20 Lines	) {
>		>
<Text style={this.props.styles.blueAction}>Add</Text>		<Text style={this.props.styles.blueAction}>Add</Text>
</TouchableOpacity>		</TouchableOpacity>
);		);
}		}

return (		return (
<View style={[this.props.styles.container, borderBottom]}>		<View style={[this.props.styles.container, borderBottom]}>
		<TouchableOpacity
		ginsuAuthorUnsubmitted Done Inline Actions The red background shows the `TouchableOpacity`: I think it would be a be a better user experience if we had the entire item be a touchable instead of just the avatar/username ginsu: The red background shows the `TouchableOpacity`: {F773403} I think it would be a be a better…
		atulUnsubmitted Not Done Inline Actions What surfaces should be "touchable" is probably something we should consider as part of the Design System (at some point) CC @ashoat @ted In this particular case, it feels a little weird that we're extending so far to the right, but on the left we go just until the left edge. I guess this is a new scenario, because previously we were just listing "static" avatars/usernames and left all the interaction to the right-justified icon. Unsolicited, but if it were up to me I think it'd make sense to give the avatar/username some sort of inset to make it clear that A. the element is interactive B. the bounds of the tap target Something like: or atul: What surfaces should be "touchable" is probably something we should consider as part of the…
		ashoatUnsubmitted Not Done Inline Actions I like @atul's idea. Curious for @ted's thoughts, both on Atul's idea and also on how we can include touchable surface / interaction design of touchables in the design system ashoat: I like @atul's idea. Curious for @ted's thoughts, both on Atul's idea and also on how we can…
		tedUnsubmitted Not Done Inline Actions In this particular case, it feels a little weird that we're extending so far to the right, but on the left we go just until the left edge. I think the touch point can extend more on the left, to where `Search` starts at the input field above. @atul @ashoat I think adding touch points to the design system makes perfect sense and can be included for both web and mobile components. Each design system component lives on its own page in Figma, which will also include small documents to specifically point out the interactive touch points. These documents can point out other component rules such as places to use this component, certain rules of how to use this component, etc. For the concepts you shared, I don't think that adding an inset is necessary to point out that the element is interactive. Adding an inset feels a bit cluttered to the overall list layout. If we do want to use an inset container for the avatar and username, then reducing the separator lines (smaller, more subtle) will help with over-cluttering. I took a quick look at Discord's username touchpoints in their list. In the video, I am tapping as far right as I can before the two action buttons. The entire width seems tappable except for the two buttons. Not saying we need to do exactly how Discord handles this interaction, I wanted to see how other apps were handling this. With that, I think the UI is fine how it is, but always open if more people think we should add an inset or interactive indicator. Thank you! ted: > In this particular case, it feels a little weird that we're extending so far to the right…
		ginsuAuthorUnsubmitted Done Inline Actions For the concepts you shared, I don't think that adding an inset is necessary to point out that the element is interactive. Adding an inset feels a bit cluttered to the overall list layout. If we do want to use an inset container for the avatar and username, then reducing the separator lines (smaller, more subtle) will help with over-cluttering. I agree with this point, I think especially since we have the edit (pencil) icon to the right it would feel strange if that didn't have an inset/obvious indicator that the icon is touchable but the avatar/username does. To polish this up, I can make the entire container a touchable or at the very least, also extend the touchable to be touching the left side of this container ginsu: > For the concepts you shared, I don't think that adding an inset is necessary to point out…
		onPress={this.onPressUser}
		style={this.props.styles.userInfoContainer}
		>
<UserAvatar size="small" userID={this.props.userInfo.id} />		<UserAvatar size="small" userID={this.props.userInfo.id} />
<SingleLine style={this.props.styles.username}>		<SingleLine style={this.props.styles.username}>
{this.props.userInfo.username}		{this.props.userInfo.username}
</SingleLine>		</SingleLine>
		</TouchableOpacity>
{editButton}		{editButton}
</View>		</View>
);		);
}		}

		onPressUser = () => {
		this.props.navigateToUserProfileBottomSheet(this.props.userInfo.id);
		};

onSelect = () => {		onSelect = () => {
const { id, username } = this.props.userInfo;		const { id, username } = this.props.userInfo;
this.props.onSelect({ id, username });		this.props.onSelect({ id, username });
};		};

visibleEntryIDs() {		visibleEntryIDs() {
const { relationshipListRoute } = this.props;		const { relationshipListRoute } = this.props;
const id = {		const id = {
▲ Show 20 Lines • Show All 85 Lines • ▼ Show 20 Lines	container: {
paddingHorizontal: 24,		paddingHorizontal: 24,
paddingVertical: 10,		paddingVertical: 10,
backgroundColor: 'panelForeground',		backgroundColor: 'panelForeground',
borderColor: 'panelForegroundBorder',		borderColor: 'panelForegroundBorder',
},		},
borderBottom: {		borderBottom: {
borderBottomWidth: 1,		borderBottomWidth: 1,
},		},
		userInfoContainer: {
		flex: 1,
		flexDirection: 'row',
		},
buttonContainer: {		buttonContainer: {
flexDirection: 'row',		flexDirection: 'row',
},		},
editButtonWithMargin: {		editButtonWithMargin: {
marginLeft: 15,		marginLeft: 15,
},		},
username: {		username: {
color: 'panelForegroundSecondaryLabel',		color: 'panelForegroundSecondaryLabel',
Show All 29 Lines	) {
);		);
const colors = useColors();		const colors = useColors();
const styles = useStyles(unboundStyles);		const styles = useStyles(unboundStyles);
const dispatchActionPromise = useDispatchActionPromise();		const dispatchActionPromise = useDispatchActionPromise();
const boundUpdateRelationships = useServerCall(updateRelationships);		const boundUpdateRelationships = useServerCall(updateRelationships);
const overlayContext = React.useContext(OverlayContext);		const overlayContext = React.useContext(OverlayContext);
const keyboardState = React.useContext(KeyboardContext);		const keyboardState = React.useContext(KeyboardContext);

		const navigateToUserProfileBottomSheet =
		useNavigateToUserProfileBottomSheet();

return (		return (
<RelationshipListItem		<RelationshipListItem
{...props}		{...props}
removeUserLoadingStatus={removeUserLoadingStatus}		removeUserLoadingStatus={removeUserLoadingStatus}
colors={colors}		colors={colors}
styles={styles}		styles={styles}
dispatchActionPromise={dispatchActionPromise}		dispatchActionPromise={dispatchActionPromise}
updateRelationships={boundUpdateRelationships}		updateRelationships={boundUpdateRelationships}
overlayContext={overlayContext}		overlayContext={overlayContext}
keyboardState={keyboardState}		keyboardState={keyboardState}
		navigateToUserProfileBottomSheet={navigateToUserProfileBottomSheet}
/>		/>
);		);
});		});

export default ConnectedRelationshipListItem;		export default ConnectedRelationshipListItem;

View Options

services/backup/blob_client/src/put_client.rs

Show First 20 Lines • Show All 42 Lines • ▼ Show 20 Lines
import {		import {
RelationshipListItemTooltipModalRouteName,		RelationshipListItemTooltipModalRouteName,
FriendListRouteName,		FriendListRouteName,
BlockListRouteName,		BlockListRouteName,
} from '../navigation/route-names.js';		} from '../navigation/route-names.js';
import { useSelector } from '../redux/redux-utils.js';		import { useSelector } from '../redux/redux-utils.js';
import { type Colors, useColors, useStyles } from '../themes/colors.js';		import { type Colors, useColors, useStyles } from '../themes/colors.js';
import type { VerticalBounds } from '../types/layout-types.js';		import type { VerticalBounds } from '../types/layout-types.js';
		import { useNavigateToUserProfileBottomSheet } from '../user-profile/user-profile-utils.js';
import Alert from '../utils/alert.js';		import Alert from '../utils/alert.js';

type BaseProps = {		type BaseProps = {
+userInfo: AccountUserInfo,		+userInfo: AccountUserInfo,
+lastListItem: boolean,		+lastListItem: boolean,
+verticalBounds: ?VerticalBounds,		+verticalBounds: ?VerticalBounds,
+relationshipListRoute: NavigationRoute<'FriendList' \| 'BlockList'>,		+relationshipListRoute: NavigationRoute<'FriendList' \| 'BlockList'>,
+navigate: RelationshipListNavigate,		+navigate: RelationshipListNavigate,
Show All 10 Lines	type Props = {
// async functions that hit server APIs		// async functions that hit server APIs
+updateRelationships: (		+updateRelationships: (
request: RelationshipRequest,		request: RelationshipRequest,
) => Promise<RelationshipErrors>,		) => Promise<RelationshipErrors>,
// withOverlayContext		// withOverlayContext
+overlayContext: ?OverlayContextType,		+overlayContext: ?OverlayContextType,
// withKeyboardState		// withKeyboardState
+keyboardState: ?KeyboardState,		+keyboardState: ?KeyboardState,
		+navigateToUserProfileBottomSheet: (userID: string) => mixed,
};		};
class RelationshipListItem extends React.PureComponent<Props> {		class RelationshipListItem extends React.PureComponent<Props> {
editButton = React.createRef<React.ElementRef<typeof View>>();		editButton = React.createRef<React.ElementRef<typeof View>>();

render() {		render() {
const {		const {
lastListItem,		lastListItem,
removeUserLoadingStatus,		removeUserLoadingStatus,
▲ Show 20 Lines • Show All 77 Lines • ▼ Show 20 Lines	) {
>		>
<Text style={this.props.styles.blueAction}>Add</Text>		<Text style={this.props.styles.blueAction}>Add</Text>
</TouchableOpacity>		</TouchableOpacity>
);		);
}		}

return (		return (
<View style={[this.props.styles.container, borderBottom]}>		<View style={[this.props.styles.container, borderBottom]}>
		<TouchableOpacity
		ginsuAuthorUnsubmitted Done Inline Actions The red background shows the `TouchableOpacity`: I think it would be a be a better user experience if we had the entire item be a touchable instead of just the avatar/username ginsu: The red background shows the `TouchableOpacity`: {F773403} I think it would be a be a better…
		atulUnsubmitted Not Done Inline Actions What surfaces should be "touchable" is probably something we should consider as part of the Design System (at some point) CC @ashoat @ted In this particular case, it feels a little weird that we're extending so far to the right, but on the left we go just until the left edge. I guess this is a new scenario, because previously we were just listing "static" avatars/usernames and left all the interaction to the right-justified icon. Unsolicited, but if it were up to me I think it'd make sense to give the avatar/username some sort of inset to make it clear that A. the element is interactive B. the bounds of the tap target Something like: or atul: What surfaces should be "touchable" is probably something we should consider as part of the…
		ashoatUnsubmitted Not Done Inline Actions I like @atul's idea. Curious for @ted's thoughts, both on Atul's idea and also on how we can include touchable surface / interaction design of touchables in the design system ashoat: I like @atul's idea. Curious for @ted's thoughts, both on Atul's idea and also on how we can…
		tedUnsubmitted Not Done Inline Actions In this particular case, it feels a little weird that we're extending so far to the right, but on the left we go just until the left edge. I think the touch point can extend more on the left, to where `Search` starts at the input field above. @atul @ashoat I think adding touch points to the design system makes perfect sense and can be included for both web and mobile components. Each design system component lives on its own page in Figma, which will also include small documents to specifically point out the interactive touch points. These documents can point out other component rules such as places to use this component, certain rules of how to use this component, etc. For the concepts you shared, I don't think that adding an inset is necessary to point out that the element is interactive. Adding an inset feels a bit cluttered to the overall list layout. If we do want to use an inset container for the avatar and username, then reducing the separator lines (smaller, more subtle) will help with over-cluttering. I took a quick look at Discord's username touchpoints in their list. In the video, I am tapping as far right as I can before the two action buttons. The entire width seems tappable except for the two buttons. Not saying we need to do exactly how Discord handles this interaction, I wanted to see how other apps were handling this. With that, I think the UI is fine how it is, but always open if more people think we should add an inset or interactive indicator. Thank you! ted: > In this particular case, it feels a little weird that we're extending so far to the right…
		ginsuAuthorUnsubmitted Done Inline Actions For the concepts you shared, I don't think that adding an inset is necessary to point out that the element is interactive. Adding an inset feels a bit cluttered to the overall list layout. If we do want to use an inset container for the avatar and username, then reducing the separator lines (smaller, more subtle) will help with over-cluttering. I agree with this point, I think especially since we have the edit (pencil) icon to the right it would feel strange if that didn't have an inset/obvious indicator that the icon is touchable but the avatar/username does. To polish this up, I can make the entire container a touchable or at the very least, also extend the touchable to be touching the left side of this container ginsu: > For the concepts you shared, I don't think that adding an inset is necessary to point out…
		onPress={this.onPressUser}
		style={this.props.styles.userInfoContainer}
		>
<UserAvatar size="small" userID={this.props.userInfo.id} />		<UserAvatar size="small" userID={this.props.userInfo.id} />
<SingleLine style={this.props.styles.username}>		<SingleLine style={this.props.styles.username}>
{this.props.userInfo.username}		{this.props.userInfo.username}
</SingleLine>		</SingleLine>
		</TouchableOpacity>
{editButton}		{editButton}
</View>		</View>
);		);
}		}

		onPressUser = () => {
		this.props.navigateToUserProfileBottomSheet(this.props.userInfo.id);
		};

onSelect = () => {		onSelect = () => {
const { id, username } = this.props.userInfo;		const { id, username } = this.props.userInfo;
this.props.onSelect({ id, username });		this.props.onSelect({ id, username });
};		};

visibleEntryIDs() {		visibleEntryIDs() {
const { relationshipListRoute } = this.props;		const { relationshipListRoute } = this.props;
const id = {		const id = {
▲ Show 20 Lines • Show All 85 Lines • ▼ Show 20 Lines	container: {
paddingHorizontal: 24,		paddingHorizontal: 24,
paddingVertical: 10,		paddingVertical: 10,
backgroundColor: 'panelForeground',		backgroundColor: 'panelForeground',
borderColor: 'panelForegroundBorder',		borderColor: 'panelForegroundBorder',
},		},
borderBottom: {		borderBottom: {
borderBottomWidth: 1,		borderBottomWidth: 1,
},		},
		userInfoContainer: {
		flex: 1,
		flexDirection: 'row',
		},
buttonContainer: {		buttonContainer: {
flexDirection: 'row',		flexDirection: 'row',
},		},
editButtonWithMargin: {		editButtonWithMargin: {
marginLeft: 15,		marginLeft: 15,
},		},
username: {		username: {
color: 'panelForegroundSecondaryLabel',		color: 'panelForegroundSecondaryLabel',
Show All 29 Lines	) {
);		);
const colors = useColors();		const colors = useColors();
const styles = useStyles(unboundStyles);		const styles = useStyles(unboundStyles);
const dispatchActionPromise = useDispatchActionPromise();		const dispatchActionPromise = useDispatchActionPromise();
const boundUpdateRelationships = useServerCall(updateRelationships);		const boundUpdateRelationships = useServerCall(updateRelationships);
const overlayContext = React.useContext(OverlayContext);		const overlayContext = React.useContext(OverlayContext);
const keyboardState = React.useContext(KeyboardContext);		const keyboardState = React.useContext(KeyboardContext);

		const navigateToUserProfileBottomSheet =
		useNavigateToUserProfileBottomSheet();

return (		return (
<RelationshipListItem		<RelationshipListItem
{...props}		{...props}
removeUserLoadingStatus={removeUserLoadingStatus}		removeUserLoadingStatus={removeUserLoadingStatus}
colors={colors}		colors={colors}
styles={styles}		styles={styles}
dispatchActionPromise={dispatchActionPromise}		dispatchActionPromise={dispatchActionPromise}
updateRelationships={boundUpdateRelationships}		updateRelationships={boundUpdateRelationships}
overlayContext={overlayContext}		overlayContext={overlayContext}
keyboardState={keyboardState}		keyboardState={keyboardState}
		navigateToUserProfileBottomSheet={navigateToUserProfileBottomSheet}
/>		/>
);		);
});		});

export default ConnectedRelationshipListItem;		export default ConnectedRelationshipListItem;

[services] Rust Integration - Backup - Rust - Multiple put clientsClosedPublicActions

Details

Diff Detail

Event Timeline

Revision ContentsChangeset List

Diff 16234

services/backup/blob_client/src/get_client.rs

services/backup/blob_client/src/lib.rs

services/backup/blob_client/src/put_client.rs

[services] Rust Integration - Backup - Rust - Multiple put clients
ClosedPublic
Actions

Revision Contents
Changeset List