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

Add a tooltip for profile avatar and online status icon #6061

Merged
merged 8 commits into from
Nov 1, 2021
Merged

Add a tooltip for profile avatar and online status icon #6061

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
4c72058
add tooltip for user avatar and status
kakajann Oct 26, 2021
aa69d3c
resolve conflicts
kakajann Oct 26, 2021
9ba543a
remove unused Tooltip import
kakajann Oct 26, 2021
bc7787a
fix minor code styling
kakajann Oct 26, 2021
e566d0f
change spanish translation for syncing
kakajann Oct 26, 2021
30546dc
trigger github actions
kakajann Oct 26, 2021
be02ee5
update Spanish translations
kakajann Oct 27, 2021
899d640
update JSDoc of userStatus function
kakajann Oct 27, 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
59 changes: 44 additions & 15 deletions src/components/AvatarWithIndicator.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ import styles from '../styles/styles';
import Icon from './Icon';
import {Sync} from './Icon/Expensicons';
import SpinningIndicatorAnimation from '../styles/animation/SpinningIndicatorAnimation';
import Tooltip from './Tooltip';
import withLocalize, {withLocalizePropTypes} from './withLocalize';

const propTypes = {
/** Is user active? */
Expand All @@ -22,12 +24,18 @@ const propTypes = {

// Whether we show the sync indicator
isSyncing: PropTypes.bool,

/** To show a tooltip on hover */
tooltipText: PropTypes.string,

...withLocalizePropTypes,
};

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

class AvatarWithIndicator extends PureComponent {
Expand Down Expand Up @@ -55,6 +63,23 @@ class AvatarWithIndicator extends PureComponent {
this.animation.stop();
}

/**
* Returns user status as text
* @returns {String}
kakajann marked this conversation as resolved.
Show resolved Hide resolved
*/
userStatus() {
if (this.props.isSyncing) {
return this.props.translate('profilePage.syncing');
}

if (this.props.isActive) {
return this.props.translate('profilePage.online');
}

if (!this.props.isActive) {
return this.props.translate('profilePage.offline');
}
}

render() {
const indicatorStyles = [
Expand All @@ -69,25 +94,29 @@ class AvatarWithIndicator extends PureComponent {
<View
style={[this.props.size === 'large' ? styles.avatarLarge : styles.sidebarAvatar]}
>
<Avatar
imageStyles={[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={6}
height={6}
/>
)}
</Animated.View>
<Tooltip text={this.props.tooltipText}>
<Avatar
imageStyles={[this.props.size === 'large' ? styles.avatarLarge : null]}
source={this.props.source}
/>
</Tooltip>
<Tooltip text={this.userStatus()} absolute>
<Animated.View style={StyleSheet.flatten(indicatorStyles)}>
{this.props.isSyncing && (
<Icon
src={Sync}
fill={themeColors.textReversed}
width={6}
height={6}
/>
)}
</Animated.View>
</Tooltip>
</View>
);
}
}

AvatarWithIndicator.defaultProps = defaultProps;
AvatarWithIndicator.propTypes = propTypes;
export default AvatarWithIndicator;
export default withLocalize(AvatarWithIndicator);
3 changes: 3 additions & 0 deletions src/languages/en.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,9 @@ export default {
setMyTimezoneAutomatically: 'Set my timezone automatically',
timezone: 'Timezone',
growlMessageOnSave: 'Your profile was successfully saved',
online: 'Online',
offline: 'Offline',
syncing: 'Syncing',
},
addSecondaryLoginPage: {
addPhoneNumber: 'Add phone number',
Expand Down
3 changes: 3 additions & 0 deletions src/languages/es.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,9 @@ export default {
setMyTimezoneAutomatically: 'Configura mi zona horaria automáticamente',
timezone: 'Zona horaria',
growlMessageOnSave: 'Tu perfil se ha guardado correctamente',
online: 'Online',
offline: 'Offline',
kakajann marked this conversation as resolved.
Show resolved Hide resolved
syncing: 'Syncing',
Copy link
Contributor

Choose a reason for hiding this comment

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

I see you have asked for the translations in Slack, well done. I have also asked internally, if we even want to translate the online/offline, since they are so common.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Only changed syncing to Sincronizando

},
addSecondaryLoginPage: {
addPhoneNumber: 'Agregar número de teléfono',
Expand Down
25 changes: 12 additions & 13 deletions src/pages/home/sidebar/SidebarLinks.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -213,19 +213,18 @@ class SidebarLinks extends React.Component {
<Icon src={MagnifyingGlass} />
</TouchableOpacity>
</Tooltip>
<Tooltip text={this.props.translate('common.settings')}>
<TouchableOpacity
accessibilityLabel={this.props.translate('sidebarScreen.buttonMySettings')}
accessibilityRole="button"
onPress={this.props.onAvatarClick}
>
<AvatarWithIndicator
source={this.props.myPersonalDetails.avatar}
isActive={this.props.network && !this.props.network.isOffline}
isSyncing={this.props.network && !this.props.network.isOffline && this.props.isSyncingData}
/>
</TouchableOpacity>
</Tooltip>
<TouchableOpacity
accessibilityLabel={this.props.translate('sidebarScreen.buttonMySettings')}
accessibilityRole="button"
onPress={this.props.onAvatarClick}
>
<AvatarWithIndicator
source={this.props.myPersonalDetails.avatar}
isActive={this.props.network && !this.props.network.isOffline}
isSyncing={this.props.network && !this.props.network.isOffline && this.props.isSyncingData}
tooltipText={`${this.props.myPersonalDetails.displayName}`}
Copy link
Contributor

Choose a reason for hiding this comment

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

Since the myPersonalDetails.displayName is string, is there any particular reason why you pass it inside the `` characters? 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Whoops, I'll fix it soon

/>
</TouchableOpacity>
</View>
<OptionsList
contentContainerStyles={[
Expand Down
1 change: 1 addition & 0 deletions src/pages/settings/InitialPage.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,7 @@ const InitialSettingsPage = ({
size="large"
source={myPersonalDetails.avatar}
isActive={network.isOffline === false}
tooltipText={myPersonalDetails.displayName}
Copy link
Contributor

Choose a reason for hiding this comment

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

@parasharrajat that is a very good point and you are right. I have overlooked this 🤦 Thank you 🙇

@kakajann could you please change this copy to say settings. That should be last change to this PR I believe.

/>
</Pressable>

Expand Down