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.

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

Format whispers/welcome messages in the report action list of each chat #17527

Merged
merged 20 commits into from
Apr 20, 2023
Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
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
2 changes: 2 additions & 0 deletions src/languages/en.js
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,7 @@ export default {
message: 'Message ',
leaveRoom: 'Leave room',
you: 'You',
youAfterPreposition: 'you',
your: 'your',
conciergeHelp: 'Please reach out to Concierge for help.',
maxParticipantsReached: ({count}) => `You've selected the maximum number (${count}) of participants.`,
Expand Down Expand Up @@ -250,6 +251,7 @@ export default {
editComment: 'Edit comment',
deleteComment: 'Delete comment',
deleteConfirmation: 'Are you sure you want to delete this comment?',
onlyVisible: 'Only visible to',
},
emojiReactions: {
addReactionTooltip: 'Add reaction',
Expand Down
2 changes: 2 additions & 0 deletions src/languages/es.js
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,7 @@ export default {
message: 'Chatear con ',
leaveRoom: 'Salir de la sala de chat',
you: 'Tú',
youAfterPreposition: 'ti',
your: 'tu',
conciergeHelp: 'Por favor, contacta con Concierge para obtener ayuda.',
maxParticipantsReached: ({count}) => `Has seleccionado el número máximo (${count}) de participantes.`,
Expand Down Expand Up @@ -249,6 +250,7 @@ export default {
editComment: 'Editar comentario',
deleteComment: 'Eliminar comentario',
deleteConfirmation: '¿Estás seguro de que quieres eliminar este comentario?',
onlyVisible: 'Visible sólo para',
},
emojiReactions: {
addReactionTooltip: 'Añadir una reacción',
Expand Down
35 changes: 35 additions & 0 deletions src/libs/ReportUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -1707,6 +1707,39 @@ function canLeaveRoom(report, isPolicyMember) {
return true;
}

/**
* Returns display names for those that can see the whisper.
* However, it returns "you" if only the current user can see it besides the person that sent it.
*
* @param {string[]} participants
* @returns {string}
*/
function getWhisperDisplayNames(participants) {
const isOneOnOneWhisper = this.isOnlyVisibleByCurrentUser(participants);

// If we removed all users, it means it's only visible to "you"
if (isOneOnOneWhisper) {
return Localize.translateLocal('common.youAfterPreposition');
}

const displayNames = [];
for (let i = 0; i < participants.length; i++) {
const login = participants[i];
displayNames.push(getDisplayNameForParticipant(login, !isOneOnOneWhisper));
}
return displayNames.join(', ');
}

/**
* Returns whether a whisper is only visible by the current user and whoever sent it.
* @param {string[]} participants
* @returns {Boolean}
*/
function isOnlyVisibleByCurrentUser(participants) {
const participantsWithoutCurrentUser = _.without(participants, sessionEmail);
return participantsWithoutCurrentUser.length === 0;
}

export {
getReportParticipantsTitle,
isReportMessageAttachment,
Expand Down Expand Up @@ -1761,6 +1794,7 @@ export {
getDisplayNameForParticipant,
isExpenseReport,
isIOUReport,
isOnlyVisibleByCurrentUser,
chatIncludesChronos,
getAvatar,
isDefaultAvatar,
Expand All @@ -1775,4 +1809,5 @@ export {
getSmallSizeAvatar,
getMoneyRequestOptions,
canRequestMoney,
getWhisperDisplayNames,
};
52 changes: 49 additions & 3 deletions src/pages/home/report/ReportActionItem.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,12 @@ import * as DeviceCapabilities from '../../../libs/DeviceCapabilities';
import MiniReportActionContextMenu from './ContextMenu/MiniReportActionContextMenu';
import * as ReportActionContextMenu from './ContextMenu/ReportActionContextMenu';
import * as ContextMenuActions from './ContextMenu/ContextMenuActions';
import {withBlockedFromConcierge, withNetwork, withReportActionsDrafts} from '../../../components/OnyxProvider';
import {
withBlockedFromConcierge,
withNetwork,
withPersonalDetails,
withReportActionsDrafts,
} from '../../../components/OnyxProvider';
import RenameAction from '../../../components/ReportActionItem/RenameAction';
import InlineSystemMessage from '../../../components/InlineSystemMessage';
import styles from '../../../styles/styles';
Expand All @@ -40,6 +45,11 @@ import ChronosOOOListActions from '../../../components/ReportActionItem/ChronosO
import ReportActionItemReactions from '../../../components/Reactions/ReportActionItemReactions';
import * as Report from '../../../libs/actions/Report';
import withLocalize from '../../../components/withLocalize';
import Icon from '../../../components/Icon';
import * as Expensicons from '../../../components/Icon/Expensicons';
import Text from '../../../components/Text';
import DisplayNames from '../../../components/DisplayNames';
import personalDetailsPropType from '../../personalDetailsPropType';

const propTypes = {
/** Report for this action */
Expand Down Expand Up @@ -69,13 +79,17 @@ const propTypes = {
/** Stores user's preferred skin tone */
preferredSkinTone: PropTypes.oneOfType([PropTypes.number, PropTypes.string]),

/** All of the personalDetails */
personalDetails: PropTypes.objectOf(personalDetailsPropType),

...windowDimensionsPropTypes,
};

const defaultProps = {
draftMessage: '',
hasOutstandingIOU: false,
preferredSkinTone: CONST.EMOJI_DEFAULT_SKIN_TONE,
personalDetails: {},
};

class ReportActionItem extends Component {
Expand Down Expand Up @@ -239,6 +253,13 @@ class ReportActionItem extends Component {
if (this.props.action.actionName === CONST.REPORT.ACTIONS.TYPE.CHRONOSOOOLIST) {
return <ChronosOOOListActions action={this.props.action} reportID={this.props.report.reportID} />;
}

const whisperedTo = lodashGet(this.props.action, 'whisperedTo', []);
const isWhisper = _.size(whisperedTo) > 0;
const isMultipleParticipant = _.size(whisperedTo) > 1;
const isOnlyVisibleByUser = isWhisper ? ReportUtils.isOnlyVisibleByCurrentUser(whisperedTo) : false;
const whisperedToPersonalDetails = isWhisper ? _.filter(this.props.personalDetails, details => _.includes(whisperedTo, details.login)) : [];
Copy link
Contributor

Choose a reason for hiding this comment

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

NAB: You might want to be careful with this line. I believe that this.props.personalDetails is an object (proptypes agrees) and this filter is turning it into an array (and the default value you've provided is also an array). Looking at ReportUtils.getDisplayNamesWithTooltips() the JSDoc says that the personal details parameter should be an object, but luckily, because all that method is doing is a _.map() and it's never accessing they object keys, the code here technically works.

I'd prefer to always keep personalDetails an object.

Copy link
Contributor

Choose a reason for hiding this comment

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

Typescript won't allow this 😄

const displayNamesWithTooltips = isWhisper ? ReportUtils.getDisplayNamesWithTooltips(whisperedToPersonalDetails, isMultipleParticipant) : [];
return (
<PressableWithSecondaryInteraction
pointerEvents={this.props.action.pendingAction === CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE ? 'none' : 'auto'}
Expand All @@ -258,6 +279,7 @@ class ReportActionItem extends Component {
<View
style={StyleUtils.getReportActionItemStyle(
hovered
|| isWhisper
Copy link
Member

Choose a reason for hiding this comment

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

Hi, this is out of scope of this PR but we caused this issue - #20451 (comment)

On native, the "New" message maker is hidden behind the whisper message because it's not transparent.

|| this.state.isContextMenuActive
|| this.props.draftMessage,
(this.props.network.isOffline && this.props.action.isLoading) || this.props.action.error,
Expand All @@ -276,14 +298,37 @@ class ReportActionItem extends Component {
errorRowStyles={[styles.ml10, styles.mr2]}
needsOffscreenAlphaCompositing={this.props.action.actionName === CONST.REPORT.ACTIONS.TYPE.IOU}
>
{isWhisper && (
<View style={[styles.flexRow, styles.pl5, styles.pt2]}>
<View style={[styles.pl6, styles.mr3]}>
<Icon src={Expensicons.Eye} small />
</View>
<Text style={[styles.chatItemMessageHeaderTimestamp]}>
{this.props.translate('reportActionContextMenu.onlyVisible')}
&nbsp;
</Text>
<DisplayNames
fullTitle={ReportUtils.getWhisperDisplayNames(whisperedTo)}
displayNamesWithTooltips={displayNamesWithTooltips}
tooltipEnabled
numberOfLines={1}
textStyles={[styles.chatItemMessageHeaderTimestamp]}
shouldUseFullTitle={isOnlyVisibleByUser}
/>
</View>
)}
{!this.props.displayAsGroup
? (
<ReportActionItemSingle action={this.props.action} showHeader={!this.props.draftMessage}>
<ReportActionItemSingle
action={this.props.action}
showHeader={!this.props.draftMessage}
wrapperStyles={[styles.chatItem, isWhisper ? styles.pt1 : null]}
>
{this.renderItemContent(hovered || this.state.isContextMenuActive)}
</ReportActionItemSingle>
)
: (
<ReportActionItemGrouped>
<ReportActionItemGrouped wrapperStyles={[styles.chatItem, isWhisper ? styles.pt1 : null]}>
{this.renderItemContent(hovered || this.state.isContextMenuActive)}
</ReportActionItemGrouped>
)}
Expand Down Expand Up @@ -318,6 +363,7 @@ export default compose(
withWindowDimensions,
withLocalize,
withNetwork(),
withPersonalDetails(),
withBlockedFromConcierge({propName: 'blockedFromConcierge'}),
withReportActionsDrafts({
propName: 'draftMessage',
Expand Down
11 changes: 10 additions & 1 deletion src/pages/home/report/ReportActionItemGrouped.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,25 @@ import styles from '../../../styles/styles';
const propTypes = {
/** Children view component for this action item */
children: PropTypes.node.isRequired,

/** Styles for the outermost View */
// eslint-disable-next-line react/forbid-prop-types
wrapperStyles: PropTypes.arrayOf(PropTypes.object),
};

const defaultProps = {
wrapperStyles: [styles.chatItem],
};

const ReportActionItemGrouped = props => (
<View style={[styles.chatItem]}>
<View style={props.wrapperStyles}>
<View style={[styles.chatItemRightGrouped]}>
{props.children}
</View>
</View>
);

ReportActionItemGrouped.propTypes = propTypes;
ReportActionItemGrouped.defaultProps = defaultProps;
ReportActionItemGrouped.displayName = 'ReportActionItemGrouped';
export default ReportActionItemGrouped;
3 changes: 3 additions & 0 deletions src/pages/home/report/reportActionPropTypes.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,4 +29,7 @@ export default {

/** Error message that's come back from the server. */
error: PropTypes.string,

/** Emails of the people to which the whisper was sent to (if any). Returns empty array if it is not a whisper */
whisperedTo: PropTypes.arrayOf(PropTypes.string),
};
4 changes: 4 additions & 0 deletions src/styles/styles.js
Original file line number Diff line number Diff line change
Expand Up @@ -3136,6 +3136,10 @@ const styles = {
textAlign: 'center',
},

whisper: {
backgroundColor: themeColors.cardBG,
},

popoverMaxWidth: {
maxWidth: 375,
},
Expand Down
4 changes: 4 additions & 0 deletions src/styles/utilities/spacing.js
Original file line number Diff line number Diff line change
Expand Up @@ -370,6 +370,10 @@ export default {
paddingLeft: 20,
},

pl6: {
paddingLeft: 24,
},

pl10: {
paddingLeft: 40,
},
Expand Down