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

Jump to bottom

Add feature to view Profile Picture in full screen #8483

Merged
merged 6 commits into from
Apr 29, 2022
Merged

Add feature to view Profile Picture in full screen #8483

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
3b49343
Add feature to view Profile Picture in full screen
sobitneupane Apr 5, 2022
7fb0b2a
Refactor AttachmentModal to use PreviewAttachmentModal and UploadAtta…
sobitneupane Apr 23, 2022
d74a1e8
Merge branch 'main' into sn_feature-view-profile-picture-fs
sobitneupane Apr 23, 2022
5f24f5d
Change TouchableWithoutFocus to PressableWithoutFocus to incorporate …
sobitneupane Apr 23, 2022
f8852d8
Refactor code to use AttachmentModal with headerTitle and allowDownlo…
sobitneupane Apr 26, 2022
e91b09a
Solve the unaligned title issue of attachment in android and ios
sobitneupane Apr 29, 2022
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
View file
Open in desktop
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
1 change: 1 addition & 0 deletions src/components/HTMLEngineProvider/HTMLRenderers/ImageRenderer.js
View file
Open in desktop
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
View file
Open in desktop
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 •
edited
Loading

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
25 changes: 20 additions & 5 deletions src/pages/DetailsPage.js
View file
Open in desktop
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
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
>
{({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
View file
Open in desktop
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