[HOLD for payment 2023-03-31] [$1000] Upload photo visible area is displayed as circle even though workspace image is square

[kavimuru]

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

---

Action Performed:

1. Open the app
2. Open settings
3. Open Workspaces
4. Open any workspace
5. Click workspace profile
6. Again click on profile and click upload photo
7. Select any image and click okay, observe profile visible area shape

Expected Result:

Shape of upload photo visible area should square like the workspace image

Actual Result:

Shape of upload photo visible area is circular even though workspace image is square

Workaround:

unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

• Android / native
• Android / Chrome
• iOS / native
• iOS / Safari
• MacOS / Chrome / Safari
• MacOS / Desktop

Version Number: 1.2.82-3
Reproducible in staging?: y
Reproducible in production?: y
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos:

workspace.image.visible.area.mp4

Recording.1673.mp4

Expensify/Expensify Issue URL:
Issue reported by: @dhanashree-sawant
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1678377109201399

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~014be76ba771d0f991
• Upwork Job ID: 1634279692537335808
• Last Price Increase: 2023-03-10

[MelvinBot]

Triggered auto assignment to @abekkala (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

[MelvinBot]

Bug0 Triage Checklist (Main S/O)

• This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
• This bug is not a duplicate report (check E/App issues and #expensify-bugs)
  • If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
• This bug is reproducible using the reproduction steps in the OP. S/O
  • If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
  • If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
• This issue is filled out as thoroughly and clearly as possible
  • Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
• I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

[MelvinBot]

Job added to Upwork: https://www.upwork.com/jobs/~014be76ba771d0f991

[MelvinBot]

Current assignee @abekkala is eligible for the External assigner, not assigning anyone new.

[MelvinBot]

Triggered auto assignment to Contributor-plus team member for initial proposal review - @parasharrajat (External)

[MelvinBot]

Triggered auto assignment to @arosiclair (External), see https://stackoverflow.com/c/expensify/questions/7972 for more details.

[PrashantMangukiya]

Proposal

Please re-state the problem that we are trying to solve in this issue.

Upload photo visible area is displayed as circle even though workspace image is square

What is the root cause of that problem?

That circle mask comes from the 'ImageCropView.js' component as shown at line 74 So image crop view component will always show circle mask by default. So this is the root cause of the problem.

App/src/components/AvatarCropModal/ImageCropView.js

Lines 73 to 75 in 704d9b3

	<View style={[containerStyle, styles.l0, styles.b0, styles.pAbsolute]}>
	<Icon src={Expensicons.ImageCropMask} width={props.containerSize} height={props.containerSize} />
	</View>

What changes do you think we should make in order to solve the problem?

We have to invent one prop that will control either circle mask shown or not. e.g. props.showImageCropMask By default it will be true. So it will not affect its behaviour and show circle mask by default during profile image etc.

Step1: WorkspaceSettingsPage.js uses AvatarWithImagePicker at line 102

App/src/pages/workspace/WorkspaceSettingsPage.js

Line 102 in 080e34b

<AvatarWithImagePicker

So we have to pass showImageCropMask as false to AvatarWithImagePicker

<AvatarWithImagePicker
  ..
  showImageCropMask={false}
/>

Step2: AvatarWithImagePicker.js component uses AvatarCropModal at line 305

App/src/components/AvatarWithImagePicker.js

Line 305 in 080e34b

<AvatarCropModal

So we have to forward showImageCropMask prop to AvatarCropModal component as shown below.

const propTypes = {
  /** Show image crop circle mask */
  showImageCropMask: PropTypes.bool,
}
const defaultProps = {
  ...
  showImageCropMask: true,
};

<AvatarCropModal
  ...
  showImageCropMask={this.props.showImageCropMask}
/>

Step3: AvatarCropModal.js component uses ImageCropView at line 332

App/src/components/AvatarCropModal/AvatarCropModal.js

Line 332 in 704d9b3

<ImageCropView

So we have to forward showImageCropMask prop to ImageCropView component as shown below.

const propTypes = {
  /** Show image crop circle mask */
  showImageCropMask: PropTypes.bool,
}
const defaultProps = {
  ...
  showImageCropMask: true,
};

<ImageCropView
  ...
  showImageCropMask={props.showImageCropMask}
/>

Step4: Within ImageCropView.js add showImageCropMask prop and render Image crop mask conditionally as shown below:

const propTypes = {
  ...
  /** Show image crop circle mask */
  showImageCropMask: PropTypes.bool,
}

const defaultProps = {
  ...
  showImageCropMask: true,
};

<View style={[containerStyle, styles.l0, styles.b0, styles.pAbsolute]}> 
  { props.showImageCropMask
      && <Icon src={Expensicons.ImageCropMask} width={props.containerSize} height={props.containerSize} />
  }
</View> 

So this way it will not show circle mask for workspace image selection, but it will keep showing circle mask during profile image selection.

What alternative solutions did you explore? (Optional)

Instead of showImageCropMask prop, we can give prop name as imageCropMaskType its value will be circle or square and default value will be circle so thereafter based on that prop we can render circle mask accordingly as shown example below. For square no need to render any mask because if we do no render circle mask then it will show square area due to parent View.

<View style={[containerStyle, styles.l0, styles.b0, styles.pAbsolute]}> 
  { props.imageCropMaskType === `circle`
      && <Icon src={Expensicons.ImageCropMask} width={props.containerSize} height={props.containerSize} />
  }
</View> 

So we have to decide props name and based on that we can render the mask. Core concept is to render mask according to our need based on the prop we decide, and we have to implement the logic accordingly withinImageCropView.js file.

[mountiny]

@parasharrajat can you please check out the proposal here? Callstack might want to work on this one if the proposal from @PrashantMangukiya wont be sufficient, thanks!

[arosiclair]

Instead of showImageCropMask prop, we can give prop name as imageCropMaskType its value will be circle or square and default value will be circle

I kind of prefer this solution if we could add a rounded square mask similar to how the output is rounded

[parasharrajat]

Checking it now.

[parasharrajat]

Questions:

1. Should we show the square mask covering the full canvas area? This way the mask outline(white colored) will cover the edge of the outer canvas (image preview area).
2. Or should we show a small square that will be centered but there will be a gap between the mask border and the surrounding Canvas?
   Something like
   

For calculations, it will depend based on the above options.

@arosiclair

[arosiclair]

1️⃣ is preferable. They should be able to select the full image (minus the rounded edges).

[parasharrajat]

@PrashantMangukiya is it possible to pass the mask as prop to the ImageCropView?

[PrashantMangukiya]

@parasharrajat Yes it is possible. I did quick implementation as below, it is working as expected.

We have to define maskImage prop within ImageCropView.js as shown below.
As we are passing vector image as a props so we can set prop types validation as shown below

const propTypes = {
    /** Image crop vector mask */
    maskImage: PropTypes.func,
}

const defaultProps = {
    maskImage: Expensicons.ImageCropMask,
};

<View style={[containerStyle, styles.l0, styles.b0, styles.pAbsolute]}>
  <Icon src={props.maskImage} width={props.containerSize} height={props.containerSize} />
</View>

So within WorkspaceSettingsPage.js we have to pass maskImage prop to 'AvatarWithImagePicker' and forward that props to child component in the tree.

<AvatarWithImagePicker
  ..
  maskImage={Expensicons.Emoji}      /** Temporary used emoji icon */
/>

Thanks.

[parasharrajat]

Ok, then @PrashantMangukiya 's proposal looks good for 1️⃣ in #15845 (comment). There won't be any calculation changes as we are already calculating things based on the full width of the container.

cc: @arosiclair

🎀 👀 🎀 C+ reviewed

[arosiclair]

Cool lets go with @PrashantMangukiya's proposal with the alternative imageCropMaskType prop. I think a new rounded square mask will need to be added (Expensicons.ImageCropSquareMask)

[MelvinBot]

Reviewing label has been removed, please complete the "BugZero Checklist".

[MelvinBot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.2.88-2 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• Add square mask for workspace image upload #15975

If no regressions arise, payment will be issued on 2023-03-31. 🎊

After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.

• External issue reporter
• Contributor that fixed the issue
• Contributor+ that helped on the issue and/or PR

As a reminder, here are the bonuses/penalties that should be applied for any External issue:

• Merged PR within 3 business days of assignment - 50% bonus
• Merged PR more than 9 business days after assignment - 50% penalty

[MelvinBot]

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

• [@parasharrajat / @arosiclair] The PR that introduced the bug has been identified. Link to the PR: Add New Default Workspace Avatars #15280
• [@parasharrajat / @arosiclair] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment: Add New Default Workspace Avatars #15280 (comment)
• [@parasharrajat / @arosiclair] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion: https://expensify.slack.com/archives/C049HHMV9SM/p1680620161477389
• [@abekkala] Determine if we should create a regression test for this bug.
• [@parasharrajat] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
• [@abekkala] Link the GH issue for creating/updating the regression test once above steps have been agreed upon: https://github.com/Expensify/Expensify/issues/273658

[PrashantMangukiya]

Ping for Upwork

[arosiclair]

Partially filled out the checklist above

[parasharrajat]

Regression test Steps

1. Open the app on plaform.
2. Go to Settings > Profile page.
3. Click on avatar and choose a photo.
4. Notice that the Avatar editor has circled the canvas area.
5. Go to Settings> Workspaces > [Select any workspace].
6. Click on avatar and choose a photo.
7. Notice that the Avatar editor has a Rounded Square canvas area.

Do you agree 👍 or 👎 ?

[PrashantMangukiya]

@parasharrajat please correct this

7. Notice that the Avatar editor has circled the canvas area

to this

7. Notice that the Avatar editor has Rounded Square canvas area

[abekkala]

Payments to be made:
Issue reported by: @dhanashree-sawant [$250]
Selected Proposal for fix: @PrashantMangukiya [$1000]
C+ Review: @parasharrajat [$1000]

Mar 14 - @PrashantMangukiya was been assigned to this job
Mar 15 - PR merged #15975
Mar 21 - Deployed to staging
Mar 24 - Deployed to production

No bonus payout

[arosiclair]

I think we can just update specifically these steps to account for the rounded square

[abekkala]

@dhanashree-sawant - invited to Upwork Job
@PrashantMangukiya - hired/sent contract in Upwork
@parasharrajat - invited to Upwork Job

Once those are accepted I can process payments

[PrashantMangukiya]

@abekkala Offer accepted on Upwork. Thank you.

[abekkala]

@PrashantMangukiya Paid and contract ended - thank you!

[dhanashree-sawant]

@abekkala Thanks, I have accepted the invite

[abekkala]

@dhanashree-sawant paid and contract ended - thank you!

[abekkala]

@parasharrajat paid and contract ended - thank you!

[abekkala]

We're all set here - closing.
Thank you everyone! 🎉

[abekkala]

@parasharrajat and @PrashantMangukiya I was actually just reviewing this and you do get the 50% bonus.
I'll send along a $500 contract for this to each of you for pay out.

(I'm sorry I was counting assignment to production days vs. assignment to merge)

[abekkala]

@parasharrajat and @PrashantMangukiya I've just created another job listing just for this bonus pay out [$500] Merge bonus for #15845 - Expensify and hired you both, once you accept I'll process payout today!

[parasharrajat]

Thanks @abekkala Accepted.

[PrashantMangukiya]

Thanks @abekkala Accepted offer.

[abekkala]

@parasharrajat and @PrashantMangukiya bonus payouts sent!

Sorry about that! 😅
All set now - Closing!