Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
23 changes: 13 additions & 10 deletions src/components/AttachmentModal.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,6 @@ import TextWithEllipsis from './TextWithEllipsis';
*/

const propTypes = {
/** Determines title of the modal header depending on if we are uploading an attachment or not */
isUploadingAttachment: PropTypes.bool,

/** Optional source URL for the image shown. If not passed in via props must be specified when modal is opened. */
sourceURL: PropTypes.string,

Expand All @@ -46,17 +43,24 @@ const propTypes = {
/** Do the urls require an authToken? */
isAuthTokenRequired: PropTypes.bool,

/** Determines if download Button should be shown or not */
allowDownload: PropTypes.bool,

/** Title shown in the header of the modal */
headerTitle: PropTypes.string,

...withLocalizePropTypes,

...windowDimensionsPropTypes,
};

const defaultProps = {
isUploadingAttachment: false,
sourceURL: null,
onConfirm: null,
originalFileName: null,
isAuthTokenRequired: false,
allowDownload: false,
headerTitle: null,
onModalHide: () => {},
};

Expand Down Expand Up @@ -145,6 +149,7 @@ class AttachmentModal extends PureComponent {
: [styles.imageModalImageCenterContainer, styles.p5];

const {fileName, fileExtension} = this.splitExtensionFromFileName();

return (
<>
<Modal
Expand All @@ -157,21 +162,19 @@ class AttachmentModal extends PureComponent {
propagateSwipe
>
<HeaderWithCloseButton
title={this.props.isUploadingAttachment
? this.props.translate('reportActionCompose.sendAttachment')
: this.props.translate('common.attachment')}
title={this.props.headerTitle || this.props.translate('common.attachment')}
shouldShowBorderBottom
shouldShowDownloadButton={!this.props.isUploadingAttachment}
shouldShowDownloadButton={this.props.allowDownload}
onDownloadButtonPress={() => fileDownload(sourceURL, this.props.originalFileName)}
onCloseButtonPress={() => this.setState({isModalOpen: false})}
subtitle={(
subtitle={fileName ? (
<TextWithEllipsis
leadingText={fileName}
trailingText={fileExtension ? `.${fileExtension}` : ''}
wrapperStyle={[styles.w100]}
textStyle={styles.mutedTextLabel}
/>
)}
) : ''}
/>
<View style={attachmentViewStyles}>
{this.state.sourceURL && (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ const ImageRenderer = (props) => {

return (
<AttachmentModal
allowDownload
sourceURL={source}
isAuthTokenRequired={isAttachment}
originalFileName={originalFileName}
Expand Down
2 changes: 2 additions & 0 deletions src/libs/actions/PersonalDetails.js
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ function formatPersonalDetails(personalDetailsList) {
const lastName = details.lastName || '';
const payPalMeAddress = details.expensify_payPalMeAddress || '';
const phoneNumber = details.phoneNumber || '';
const avatarHighResolution = details.avatar || details.avatarThumbnail;
formattedResult[sanitizedLogin] = {
login: sanitizedLogin,
avatar,
Expand All @@ -109,6 +110,7 @@ function formatPersonalDetails(personalDetailsList) {
timezone,
payPalMeAddress,
phoneNumber,
avatarHighResolution,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think it would be better to use the avatarThumbnail and avatar properties. which means use avatarThumbnail instead of avatar and store details.avatar in the avatar.

Now use avatarThumbnail where the avatar was used.

@sobitneupane sobitneupane Apr 26, 2022

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Now use avatarThumbnail where the avatar was used.

@parasharrajat Do we want to do this change here? This might require massive code refactoring.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I was expecting it to be done here because of the keys. But I don't mind another PR doing that. At last, I am fine with this as well if @Julesssss is.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yeah, I think that's okay.

};
});
Timing.end(CONST.TIMING.PERSONAL_DETAILS_FORMATTED);
Expand Down
25 changes: 20 additions & 5 deletions src/pages/DetailsPage.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ import * as ReportUtils from '../libs/reportUtils';
import DateUtils from '../libs/DateUtils';
import * as Expensicons from '../components/Icon/Expensicons';
import MenuItem from '../components/MenuItem';
import AttachmentModal from '../components/AttachmentModal';
import PressableWithoutFocus from '../components/PressableWithoutFocus';
import * as Report from '../libs/actions/Report';

const matchType = PropTypes.shape({
Expand Down Expand Up @@ -98,11 +100,24 @@ const DetailsPage = (props) => {
{details ? (
<ScrollView>
<View style={styles.pageWrapper}>
<Avatar
containerStyles={[styles.avatarLarge, styles.mb3]}
imageStyles={[styles.avatarLarge]}
source={details.avatar}
/>
<AttachmentModal
headerTitle={isSMSLogin ? props.toLocalPhone(details.displayName) : details.displayName}
sourceURL={details.avatarHighResolution}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can't we do that directly here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

details.avatar being used here is avatar returned from formatPersonalDetails function which is of lower resolution. So, new property for higher resolution is added in formatPersonalDetails function which is later used here.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Instead of this directly use sourceURL={details.avatar || details.avatarThumbnail} here

isAuthTokenRequired
>
{({show}) => (
<PressableWithoutFocus
style={styles.noOutline}
onPress={show}
>
<Avatar
containerStyles={[styles.avatarLarge, styles.mb3]}
imageStyles={[styles.avatarLarge]}
source={details.avatar}
/>
</PressableWithoutFocus>
)}
</AttachmentModal>
{details.displayName && (
<Text style={[styles.displayName, styles.mb6]} numberOfLines={1}>
{isSMSLogin ? props.toLocalPhone(details.displayName) : details.displayName}
Expand Down
2 changes: 1 addition & 1 deletion src/pages/home/report/ReportActionCompose.js
Original file line number Diff line number Diff line change
Expand Up @@ -414,7 +414,7 @@ class ReportActionCompose extends React.Component {
]}
>
<AttachmentModal
isUploadingAttachment
headerTitle={this.props.translate('reportActionCompose.sendAttachment')}
onConfirm={(file) => {
this.submitForm();
Report.addAction(this.props.reportID, '', file);
Expand Down