-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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.
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
fix: remove getDisplayNameForTypingIndicator #31495
Changes from 11 commits
a57d229
9093ff3
6358b1e
1ff3017
b30cb29
921a070
2ffcec2
5fec558
fe1e320
e7d667c
8af4811
9d26c00
c0a9c6a
e6ed623
e186fd3
3ec5e40
88cf035
31df21e
8fb1f2f
69ba3a0
c1435f4
a4e422f
10f49fa
1bab5a9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -2,80 +2,74 @@ import PropTypes from 'prop-types'; | |||||
import React, {useMemo} from 'react'; | ||||||
import {withOnyx} from 'react-native-onyx'; | ||||||
import _ from 'underscore'; | ||||||
import networkPropTypes from '@components/networkPropTypes'; | ||||||
import {withNetwork} from '@components/OnyxProvider'; | ||||||
import Text from '@components/Text'; | ||||||
import TextWithEllipsis from '@components/TextWithEllipsis'; | ||||||
import withLocalize, {withLocalizePropTypes} from '@components/withLocalize'; | ||||||
import compose from '@libs/compose'; | ||||||
import useLocalize from '@hooks/useLocalize'; | ||||||
import useNetwork from '@hooks/useNetwork'; | ||||||
import * as PersonalDetailsUtils from '@libs/PersonalDetailsUtils'; | ||||||
import * as ReportUtils from '@libs/ReportUtils'; | ||||||
import useThemeStyles from '@styles/useThemeStyles'; | ||||||
import * as PersonalDetails from '@userActions/PersonalDetails'; | ||||||
import ONYXKEYS from '@src/ONYXKEYS'; | ||||||
|
||||||
const propTypes = { | ||||||
/** Key-value pairs of user accountIDs/logins and whether or not they are typing. Keys are accountIDs or logins. */ | ||||||
userTypingStatuses: PropTypes.objectOf(PropTypes.bool), | ||||||
|
||||||
/** Information about the network */ | ||||||
network: networkPropTypes.isRequired, | ||||||
|
||||||
...withLocalizePropTypes, | ||||||
}; | ||||||
|
||||||
const defaultProps = { | ||||||
userTypingStatuses: {}, | ||||||
}; | ||||||
|
||||||
function ReportTypingIndicator(props) { | ||||||
function ReportTypingIndicator({userTypingStatuses}) { | ||||||
const {translate} = useLocalize(); | ||||||
const {isOffline} = useNetwork(); | ||||||
|
||||||
const styles = useThemeStyles(); | ||||||
const usersTyping = useMemo(() => _.filter(_.keys(props.userTypingStatuses), (loginOrAccountID) => props.userTypingStatuses[loginOrAccountID]), [props.userTypingStatuses]); | ||||||
const usersTyping = useMemo(() => _.filter(_.keys(userTypingStatuses), (loginOrAccountID) => userTypingStatuses[loginOrAccountID]), [userTypingStatuses]); | ||||||
const firstUserTyping = usersTyping[0]; | ||||||
|
||||||
const firstUserTypingID = useMemo( | ||||||
() => (firstUserTyping && Number.isNaN(Number(firstUserTyping)) ? PersonalDetailsUtils.getAccountIDsByLogins([firstUserTyping])[0] : firstUserTyping), | ||||||
[firstUserTyping], | ||||||
); | ||||||
|
||||||
// If we are offline, the user typing statuses are not up-to-date so do not show them | ||||||
if (props.network.isOffline) { | ||||||
if (isOffline || !firstUserTyping) { | ||||||
return null; | ||||||
} | ||||||
|
||||||
const numUsersTyping = _.size(usersTyping); | ||||||
|
||||||
// Decide on the Text element that will hold the display based on the number of users that are typing. | ||||||
switch (numUsersTyping) { | ||||||
case 0: | ||||||
return null; | ||||||
|
||||||
case 1: | ||||||
return ( | ||||||
<TextWithEllipsis | ||||||
leadingText={PersonalDetails.getDisplayNameForTypingIndicator(usersTyping[0], props.translate('common.someone'))} | ||||||
trailingText={` ${props.translate('reportTypingIndicator.isTyping')}`} | ||||||
textStyle={[styles.chatItemComposeSecondaryRowSubText]} | ||||||
wrapperStyle={[styles.chatItemComposeSecondaryRow, styles.flex1]} | ||||||
leadingTextParentStyle={styles.chatItemComposeSecondaryRowOffset} | ||||||
/> | ||||||
); | ||||||
// If the user is typing on OldDot, firstUserTyping will be a string (the user's login) | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
const firstUserTypingDisplayName = ReportUtils.getDisplayNameForParticipant(firstUserTypingID, false, false) || firstUserTyping; | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think this will work when users don't know each other. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have 2 solutions in mind:
I prefer 1st one since user can still set any random email as display name There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @situchan can even such a scenario occur? How could I reproduce it? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. which one are you not able to reproduce? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here's repro step:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please don't set display name on both accounts and try again There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think the purpose of hidden logic is to secure sensitive info ( There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. but for one of them it was set automatically There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe you logged with google? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I unset it, still the same. I'll get back to it on Monday Screen.Recording.2023-12-01.at.18.40.35.mp4 |
||||||
|
||||||
default: | ||||||
return ( | ||||||
<Text | ||||||
style={[styles.chatItemComposeSecondaryRowSubText, styles.chatItemComposeSecondaryRowOffset]} | ||||||
numberOfLines={1} | ||||||
> | ||||||
{props.translate('reportTypingIndicator.multipleUsers')} | ||||||
{` ${props.translate('reportTypingIndicator.areTyping')}`} | ||||||
</Text> | ||||||
); | ||||||
if (usersTyping.length === 1) { | ||||||
return ( | ||||||
<TextWithEllipsis | ||||||
leadingText={firstUserTypingDisplayName || translate('common.someone')} | ||||||
trailingText={` ${translate('reportTypingIndicator.isTyping')}`} | ||||||
textStyle={[styles.chatItemComposeSecondaryRowSubText]} | ||||||
wrapperStyle={[styles.chatItemComposeSecondaryRow, styles.flex1]} | ||||||
leadingTextParentStyle={styles.chatItemComposeSecondaryRowOffset} | ||||||
/> | ||||||
); | ||||||
} | ||||||
return ( | ||||||
<Text | ||||||
style={[styles.chatItemComposeSecondaryRowSubText, styles.chatItemComposeSecondaryRowOffset]} | ||||||
numberOfLines={1} | ||||||
> | ||||||
{translate('reportTypingIndicator.multipleUsers')} | ||||||
{` ${translate('reportTypingIndicator.areTyping')}`} | ||||||
</Text> | ||||||
); | ||||||
} | ||||||
|
||||||
ReportTypingIndicator.propTypes = propTypes; | ||||||
ReportTypingIndicator.defaultProps = defaultProps; | ||||||
ReportTypingIndicator.displayName = 'ReportTypingIndicator'; | ||||||
|
||||||
export default compose( | ||||||
withLocalize, | ||||||
withNetwork(), | ||||||
withOnyx({ | ||||||
userTypingStatuses: { | ||||||
key: ({reportID}) => `${ONYXKEYS.COLLECTION.REPORT_USER_IS_TYPING}${reportID}`, | ||||||
initialValue: {}, | ||||||
}, | ||||||
}), | ||||||
)(ReportTypingIndicator); | ||||||
export default withOnyx({ | ||||||
userTypingStatuses: { | ||||||
key: ({reportID}) => `${ONYXKEYS.COLLECTION.REPORT_USER_IS_TYPING}${reportID}`, | ||||||
initialValue: {}, | ||||||
}, | ||||||
})(ReportTypingIndicator); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please move old comment to here