Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Added sync indicator #2810

Merged
merged 12 commits into from
May 21, 2021
Merged

Added sync indicator #2810

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
ec80ff2
new: added sync indicator
parasharrajat May 11, 2021
e37ee15
animation on native
parasharrajat May 11, 2021
ae3858c
typo
parasharrajat May 11, 2021
a3baacd
Update src/pages/home/sidebar/SidebarLinks.js
parasharrajat May 13, 2021
78a084b
Update src/components/AvatarWithIndicator.js
parasharrajat May 13, 2021
fcf5b8a
Pull latest changes
parasharrajat May 14, 2021
fa3ae71
fix: animation issue with indicator
parasharrajat May 14, 2021
743ee68
Pull remote changes
parasharrajat May 14, 2021
4ebe4be
pull latest changes
parasharrajat May 16, 2021
293375b
fix: sync icon is visible after user is offline
parasharrajat May 16, 2021
8aca3f2
chore: animation timing
parasharrajat May 18, 2021
92addf9
merge Conflicts
parasharrajat May 21, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions assets/images/sync.svg
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
134 changes: 109 additions & 25 deletions src/components/AvatarWithIndicator.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,13 @@
import React, {memo} from 'react';
import {View, StyleSheet} from 'react-native';
import React, {PureComponent} from 'react';
import {
View, StyleSheet, Animated, Easing,
} from 'react-native';
import PropTypes from 'prop-types';
import Avatar from './Avatar';
import themeColors from '../styles/themes/default';
import styles from '../styles/styles';
import Icon from './Icon';
import {Sync} from './Icon/Expensicons';

const propTypes = {
// Is user active?
Expand All @@ -14,37 +19,116 @@ const propTypes = {
// avatar size
size: PropTypes.string,

// When true, shows sync indicator
parasharrajat marked this conversation as resolved.
Show resolved Hide resolved
isSyncing: PropTypes.bool,
};

const defaultProps = {
isActive: false,
size: 'default',
isSyncing: false,
};

const AvatarWithIndicator = ({
isActive,
source,
size,
}) => {
const indicatorStyles = [
size === 'large' ? styles.statusIndicatorLarge : styles.statusIndicator,
isActive ? styles.statusIndicatorOnline : styles.statusIndicatorOffline,
];

return (
<View
style={[size === 'large' ? styles.avatarLarge : styles.sidebarAvatar]}
>
<Avatar
style={[size === 'large' ? styles.avatarLarge : null]}
source={source}
/>
<View style={StyleSheet.flatten(indicatorStyles)} />
</View>
);
};
class AvatarWithIndicator extends PureComponent {
constructor(props) {
super(props);

this.rotate = new Animated.Value(0);
this.scale = new Animated.Value(1);
this.startSyncIndicator = this.startSyncIndicator.bind(this);
this.stopSyncIndicator = this.stopSyncIndicator.bind(this);
}

componentDidMount() {
if (this.props.isSyncing) {
this.startSyncIndicator();
}
}

componentDidUpdate(prevProps) {
parasharrajat marked this conversation as resolved.
Show resolved Hide resolved
if (!prevProps.isSyncing && this.props.isSyncing) {
this.startSyncIndicator();
} else if (prevProps.isSyncing && !this.props.isSyncing) {
this.stopSyncIndicator();
}
}

componentWillUnmount() {
this.stopSyncIndicator();
}

startSyncIndicator() {
Animated.loop(Animated.timing(this.rotate, {
toValue: 1,
duration: 3000,
easing: Easing.linear,
isInteraction: false,
useNativeDriver: true,
})).start();
Animated.spring(this.scale, {
toValue: 1.35,
tension: 1,
isInteraction: false,
useNativeDriver: true,
}).start();
}

stopSyncIndicator() {
Animated.spring(this.scale, {
toValue: 1,
tension: 1,
isInteraction: false,
useNativeDriver: true,
}).start(() => {
this.rotate.resetAnimation();
this.scale.resetAnimation();
this.rotate.setValue(0);
});
}

render() {
const indicatorStyles = [
styles.alignItemsCenter,
styles.justifyContentCenter,
{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We've tried to take a pretty hard stance against inline styles. In this scenario, we would usually make a function in styles.js called getAvatarIndicatorStyles or something and dynamically generate these styles in there. You can see how I handed transforms in a dynamic style function in src/styles/getTooltipSyles.js. We haven't really decided whether dynamic style generators should be in their own module or directly in styles.js, so it's up to you! Since it's probably just one function, I would probably put it directly in styles.js

transform: [{
rotate: this.rotate.interpolate({
inputRange: [0, 1],
outputRange: ['0deg', '-360deg'],
}),
}, {
scale: this.scale,
}],
},
this.props.isSyncing ? styles.statusIndicatorSyncing : null,
this.props.size === 'large' ? styles.statusIndicatorLarge : styles.statusIndicator,
this.props.isActive ? styles.statusIndicatorOnline : styles.statusIndicatorOffline,
];

return (
<View
style={[this.props.size === 'large' ? styles.avatarLarge : styles.sidebarAvatar]}
>
<Avatar
style={[this.props.size === 'large' ? styles.avatarLarge : null]}
source={this.props.source}
/>
<Animated.View style={StyleSheet.flatten(indicatorStyles)}>
{this.props.isSyncing && (
<Icon
src={Sync}
fill={themeColors.textReversed}
width="100%"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you try making the icon 12x12? This way there is a little bit of margin around the icon inside of the 16x16 green circle.

image

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@shawnborton But indicator size is 12x12 including 2px transparent border around. So Green circle is 8x8.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We want the indicator to become larger when we are showing that content is syncing. So the green online indicator should grow in size and then show the spinning icon inside of it. Let me know if that makes sense - happy to make a prototype as well.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK after growing, it should be 16px. makes sense.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@shawnborton It looks a little big. Don't you think so?
image

Copy link
Member Author

@parasharrajat parasharrajat May 13, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or if this is looking fine, let me know I will update the PR.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@shawnborton any thoughts?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree it looks big. I think ideally the size of the indicator wouldn't need to change – we just show the spinning icon in the normal-size indicator if it's loading.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can try a smaller size, which would be a green circle of 12x12, with the icon inside of it at 8x8. It would look like this:
image

image

height="100%"
/>
)}
</Animated.View>
</View>
);
}
}

AvatarWithIndicator.defaultProps = defaultProps;
AvatarWithIndicator.propTypes = propTypes;
AvatarWithIndicator.displayName = 'AvatarWithIndicator';
export default memo(AvatarWithIndicator);
export default AvatarWithIndicator;
TomatoToaster marked this conversation as resolved.
Show resolved Hide resolved
2 changes: 2 additions & 0 deletions src/components/Icon/Expensicons.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import Camera from '../../../assets/images/camera.svg';
import Gallery from '../../../assets/images/gallery.svg';
import Offline from '../../../assets/images/offline.svg';
import SignOut from '../../../assets/images/sign-out.svg';
import Sync from '../../../assets/images/sync.svg';

export {
ArrowRight,
Expand Down Expand Up @@ -64,4 +65,5 @@ export {
Users,
Wallet,
SignOut,
Sync,
};
4 changes: 2 additions & 2 deletions src/components/Icon/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,10 @@ const propTypes = {
src: PropTypes.func.isRequired,

// The width of the icon.
width: PropTypes.number,
width: PropTypes.oneOfType([PropTypes.number, PropTypes.string]),

// The height of the icon.
height: PropTypes.number,
height: PropTypes.oneOfType([PropTypes.number, PropTypes.string]),

// The fill color for the icon.
// Can be provided in hex, rgb, rgba, or as a valid react-native named color such as 'red' or 'blue'.
Expand Down
8 changes: 8 additions & 0 deletions src/pages/home/sidebar/SidebarLinks.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,9 @@ const propTypes = {

// Whether we have the necessary report data to load the sidebar
initialReportDataLoaded: PropTypes.bool,

// Syncing App Data
parasharrajat marked this conversation as resolved.
Show resolved Hide resolved
isSyncingData: PropTypes.bool,
};

const defaultProps = {
Expand All @@ -84,6 +87,7 @@ const defaultProps = {
currentlyViewedReportID: '',
priorityMode: CONST.PRIORITY_MODE.DEFAULT,
initialReportDataLoaded: false,
isSyncingData: false,
};

class SidebarLinks extends React.Component {
Expand Down Expand Up @@ -143,6 +147,7 @@ class SidebarLinks extends React.Component {
<AvatarWithIndicator
source={this.props.myPersonalDetails.avatar}
isActive={this.props.network && !this.props.network.isOffline}
isSyncing={this.props.isSyncingData}
/>
</TouchableOpacity>
</View>
Expand Down Expand Up @@ -200,5 +205,8 @@ export default compose(
initialReportDataLoaded: {
key: ONYXKEYS.INITIAL_REPORT_DATA_LOADED,
},
isSyncingData: {
key: ONYXKEYS.IS_LOADING_AFTER_RECONNECT,
},
}),
)(SidebarLinks);
29 changes: 17 additions & 12 deletions src/styles/styles.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -457,6 +457,23 @@ const styles = {
width: 12,
zIndex: 10,
},

statusIndicatorLarge: {
borderColor: themeColors.componentBG,
borderRadius: 8,
borderWidth: 2,
position: 'absolute',
right: 4,
bottom: 4,
height: 16,
width: 16,
zIndex: 10,
},

statusIndicatorSyncing: {
padding: 1,
},

statusIndicatorOnline: {
backgroundColor: themeColors.online,
},
Expand Down Expand Up @@ -1175,18 +1192,6 @@ const styles = {
height: 80,
},

statusIndicatorLarge: {
borderColor: themeColors.componentBG,
borderRadius: 8,
borderWidth: 2,
position: 'absolute',
right: 4,
bottom: 4,
height: 16,
width: 16,
zIndex: 10,
},

displayName: {
fontSize: variables.fontSizeLarge,
fontFamily: fontFamily.GTA_BOLD,
Expand Down