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

Add feature to view Profile Picture in full screen #8483

Merged
19 changes: 11 additions & 8 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,11 +162,9 @@ class AttachmentModal extends PureComponent {
propagateSwipe
>
<HeaderWithCloseButton
title={this.props.isUploadingAttachment
? this.props.translate('reportActionCompose.sendAttachment')
: this.props.translate('common.attachment')}
title={this.props.headerTitle}
Copy link
Member

Choose a reason for hiding this comment

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

this should be like this.

Suggested change
title={this.props.headerTitle}
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={(
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import React from 'react';
import htmlRendererPropTypes from './htmlRendererPropTypes';
import Config from '../../../CONFIG';
import AttachmentModal from '../../AttachmentModal';
import PreviewAttachmentModal from '../../PreviewAttachmentModal';
import styles from '../../../styles/styles';
import ThumbnailImage from '../../ThumbnailImage';
import PressableWithoutFocus from '../../PressableWithoutFocus';
Expand Down Expand Up @@ -45,7 +45,7 @@ const ImageRenderer = (props) => {
);

return (
<AttachmentModal
<PreviewAttachmentModal
sourceURL={source}
isAuthTokenRequired={isAttachment}
originalFileName={originalFileName}
Expand All @@ -62,7 +62,7 @@ const ImageRenderer = (props) => {
/>
</PressableWithoutFocus>
)}
</AttachmentModal>
</PreviewAttachmentModal>
);
};

Expand Down
37 changes: 37 additions & 0 deletions src/components/PreviewAttachmentModal.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
import _ from 'underscore';
import React from 'react';
import PropTypes from 'prop-types';
import AttachmentModal from './AttachmentModal';
import withLocalize, {withLocalizePropTypes} from './withLocalize';

const propTypes = {
/** Determines title of the modal header depending on if we are viewing a profile picture or not */
isProfilePicture: PropTypes.bool,

/** DisplayName to be used as headerTitle if isProfilePicture is true. */
displayName: PropTypes.string,

...withLocalizePropTypes,
};

const defaultProps = {
isProfilePicture: false,
displayName: null,
};

const PreviewAttachmentModal = (props) => {
const propsToPass = _.omit(props, 'displayName', 'isProfilePicture');
return (
<AttachmentModal
headerTitle={props.isProfilePicture ? props.displayName : props.translate('common.attachment')}
allowDownload={!props.isProfilePicture}
/* eslint-disable-next-line react/jsx-props-no-spreading */
{...propsToPass}
/>
Copy link
Member

Choose a reason for hiding this comment

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

Lets remove this component and directly use AttachmentModal and set the headerTitle & allowDownload accordingly.

);
};

PreviewAttachmentModal.propTypes = propTypes;
PreviewAttachmentModal.defaultProps = defaultProps;
PreviewAttachmentModal.displayName = 'PreviewAttachmentModal';
export default withLocalize(PreviewAttachmentModal);
19 changes: 19 additions & 0 deletions src/components/UploadAttachmentModal.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
import React from 'react';
import AttachmentModal from './AttachmentModal';
import withLocalize, {withLocalizePropTypes} from './withLocalize';

const propTypes = {
...withLocalizePropTypes,
};

const UploadAttachmentModal = props => (
Copy link
Member

Choose a reason for hiding this comment

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

Lets remove this component and directly use AttachmentModal and set the headerTitle & allowDownload accordingly.

These components are not adding much value.

<AttachmentModal
headerTitle={props.translate('reportActionCompose.sendAttachment')}
/* eslint-disable-next-line react/jsx-props-no-spreading */
{...props}
/>
);

UploadAttachmentModal.propTypes = propTypes;
UploadAttachmentModal.displayName = 'UploadAttachmentModal';
export default withLocalize(UploadAttachmentModal);
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
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.

Copy link
Contributor Author

@sobitneupane sobitneupane Apr 26, 2022

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
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
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
26 changes: 21 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 PreviewAttachmentModal from '../components/PreviewAttachmentModal';
import PressableWithoutFocus from '../components/PressableWithoutFocus';
import * as Report from '../libs/actions/Report';

const matchType = PropTypes.shape({
Expand Down Expand Up @@ -98,11 +100,25 @@ const DetailsPage = (props) => {
{details ? (
<ScrollView>
<View style={styles.pageWrapper}>
<Avatar
containerStyles={[styles.avatarLarge, styles.mb3]}
imageStyles={[styles.avatarLarge]}
source={details.avatar}
/>
<PreviewAttachmentModal
sourceURL={details.avatarHighResolution}
Copy link
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
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
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
displayName={isSMSLogin ? props.toLocalPhone(details.displayName) : details.displayName}
isProfilePicture
>
{({show}) => (
<PressableWithoutFocus
style={styles.noOutline}
onPress={show}
>
<Avatar
containerStyles={[styles.avatarLarge, styles.mb3]}
imageStyles={[styles.avatarLarge]}
source={details.avatar}
/>
</PressableWithoutFocus>
)}
</PreviewAttachmentModal>
{details.displayName && (
<Text style={[styles.displayName, styles.mb6]} numberOfLines={1}>
{isSMSLogin ? props.toLocalPhone(details.displayName) : details.displayName}
Expand Down
6 changes: 3 additions & 3 deletions src/pages/home/report/ReportActionCompose.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import * as Expensicons from '../../../components/Icon/Expensicons';
import AttachmentPicker from '../../../components/AttachmentPicker';
import * as Report from '../../../libs/actions/Report';
import ReportTypingIndicator from './ReportTypingIndicator';
import AttachmentModal from '../../../components/AttachmentModal';
import UploadAttachmentModal from '../../../components/UploadAttachmentModal';
import compose from '../../../libs/compose';
import PopoverMenu from '../../../components/PopoverMenu';
import withWindowDimensions, {windowDimensionsPropTypes} from '../../../components/withWindowDimensions';
Expand Down Expand Up @@ -413,7 +413,7 @@ class ReportActionCompose extends React.Component {
styles.flexRow,
]}
>
<AttachmentModal
<UploadAttachmentModal
isUploadingAttachment
onConfirm={(file) => {
this.submitForm();
Expand Down Expand Up @@ -552,7 +552,7 @@ class ReportActionCompose extends React.Component {
/>
</>
)}
</AttachmentModal>
</UploadAttachmentModal>
{canUseTouchScreen() && this.props.isMediumScreenWidth ? null : (
<EmojiPickerButton
isDisabled={isBlockedFromConcierge}
Expand Down