-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
[HOLD for payment 2021-12-06] Room details - Room avatar is square instead of circle #6288
Comments
Triggered auto assignment to @tylerkaraszewski ( |
Android has circled image. |
Seems like a straightforward contributor issue, I'll tag as such and let someone propose a solution. |
Triggered auto assignment to @bfitzexpensify ( |
Upwork posting. |
Triggered auto assignment to @Jag96 ( |
@isagoico Could you please confirm if this is reproducible on any platform? my testing revealed that it looks good on the latest code. |
Proposed SolutionThis is reproducible in version: 1.1.14-4 for iOS 15.0 Issue-Reproducible-ios.movWe need to change style at line 134, 135, and 152 within ReportDetailsPage.js file. App/src/pages/ReportDetailsPage.js Lines 131 to 137 in 6701446
App/src/pages/ReportDetailsPage.js Lines 147 to 157 in 6701446
**Solution: ** Update style as shown under. <Avatar
isDefaultChatRoom={isDefaultRoom(this.props.report)}
isArchivedRoom={isArchivedRoom(this.props.report)}
containerStyles={[styles.avatarLarge, styles.mb4]} // *** Updated code here
imageStyles={[styles.avatarLarge]} // *** Updated code here
source={{uri: this.props.report.icons[0]}}
/>
<Text
style={[
styles.sidebarLinkText,
styles.optionAlternateText,
styles.textLabelSupporting,
styles.mb4, // *** Updated code here
]}
numberOfLines={1}
>
{defaultRoomSubtitle}
</Text> Screen record after update iOSiOS.movAndroidAndroid.movMobile WebMobileWeb.mov |
I am in confusion @PrashantMangukiya What exactly did you change here? |
Changed style from this containerStyles={[styles.singleAvatarLarge, styles.mb4]}
imageStyles={[styles.singleAvatarLarge]} to this containerStyles={[styles.avatarLarge, styles.mb4]} // *** Updated
imageStyles={[styles.avatarLarge]} // *** Updated |
I was able to reproduce this issue in v1.1.14-0, iOS 15.0. Here is screenshots. The problem is that borderRadius property is set but overflow property is not set for the image container. The solution is, we need to add the style at line 1323 in src/styles/styles.js singleAvatarLarge: {
height: 64,
width: 64,
backgroundColor: themeColors.icon,
borderRadius: 64,
+ overflow: 'hidden', // add this line
}, We need to add |
While both proposals seem to fix the issue, I think @wenge8n has the simpler solution. @bfitzexpensify let's hire @wenge8n for this one. @wenge8n once you get the invite feel free to start on a PR! |
@wenge8n offer sent! |
The solution for this issue has been 🚀 deployed to production 🚀 in version 1.1.16-10 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue: If no regressions arise, payment will be issued on 2021-12-06. 🎊 |
Paid and contract ended. |
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:
Expected Result:
Avatar image should be a circle
Actual Result:
Avatar image is squared
Workaround:
None needed, visual issue.
Platform:
Where is this issue occurring?
Version Number: 1.1.14-0
Reproducible in staging?: Yes
Reproducible in production?: Yes
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation
Expensify/Expensify Issue URL:
Issue reported by: @shawnborton
Slack conversation: https://expensify.slack.com/archives/C01GTK53T8Q/p1636726932159700
View all open jobs on GitHub
The text was updated successfully, but these errors were encountered: