[HOLD for payment 2023-08-28] [$1000] not able to show preview of avatr in thread #21073
Comments
|
Triggered auto assignment to @kevinksullivan ( |
Bug0 Triage Checklist (Main S/O)
|
Proposed solutionPlease re-state the problem that we are trying to solve in this issue.Not able to show preview of avatar in thread What is the root cause of that problem? On details page, https://github.com/Expensify/App/blob/main/src/components/RoomHeaderAvatars.js#L26-L37 What changes do you think we should make in order to solve the problem?
const displayName = props.icons[0].type === CONST.ICON_TYPE_AVATAR ? getDisplayNameForParticipant(props.icons[0].id) : props.icons[0].name
return (
<AttachmentModal
headerTitle={props.icons[0].name}
source={UserUtils.getFullSizeAvatar(props.icons[0].source, props.icons[0].id)}
isAuthTokenRequired
>
{({ show }) => (
<PressableWithoutFocus
onPress={show}
accessibilityRole="imagebutton"
accessibilityLabel={props.icons[0].name}>
<Avatar
source={props.icons[0].source}
imageStyles={[styles.avatarLarge]}
fill={themeColors.iconSuccessFill}
size={CONST.AVATAR_SIZE.LARGE}
name={props.icons[0].name}
type={props.icons[0].type}
/>
</PressableWithoutFocus>
)}
</AttachmentModal>
);
<View pointerEvents="box-none">
<View style={[styles.flexRow, styles.wAuto, styles.ml3]}>
{_.map(iconsToDisplay, (icon, index) => {
const displayName = icon.type === CONST.ICON_TYPE_AVATAR ? getDisplayNameForParticipant(icon.id) : icon.name
return (
<View
key={`${icon.source}${index}`}
style={[styles.justifyContentCenter, styles.alignItemsCenter]}
>
<AttachmentModal
headerTitle={icon.name}
source={UserUtils.getFullSizeAvatar(icon.source, details.accountID)}
isAuthTokenRequired
originalFileName={lodashGet(details, 'originalFileName', '')}
>
{({ show }) => (
<PressableWithoutFocus
onPress={show}
accessibilityRole="imagebutton"
accessibilityLabel={icon.name}>
<Avatar
source={icon.source}
fill={themeColors.iconSuccessFill}
size={CONST.AVATAR_SIZE.LARGE}
containerStyles={[...iconStyle, StyleUtils.getAvatarBorderRadius(CONST.AVATAR_SIZE.LARGE_BORDERED, icon.type)]}
name={icon.name}
type={icon.type}
/>
</PressableWithoutFocus>
)}
</AttachmentModal>
{index === CONST.REPORT.MAX_PREVIEW_AVATARS - 1 && props.icons.length - CONST.REPORT.MAX_PREVIEW_AVATARS !== 0 && (
<>
<View
style={[
styles.roomHeaderAvatarSize,
styles.roomHeaderAvatar,
...iconStyle,
StyleUtils.getAvatarBorderRadius(CONST.AVATAR_SIZE.LARGE_BORDERED, icon.type),
styles.roomHeaderAvatarOverlay,
]}
/>
<Text
style={styles.avatarInnerTextChat}>{`+${props.icons.length - CONST.REPORT.MAX_PREVIEW_AVATARS}`}</Text>
</>
)}
</View>
)
})}
</View>
</View>What alternative solutions did you explore? (Optional)N/A |
ProposalPlease re-state the problem that we are trying to solve in this issue.Preview is not implemented for avatars in thread What is the root cause of that problem?There is no implementation for avatar preview in both single and multiple avatars like on other places that show preview for the avatars. in this case it's just rendering the icons. App/src/components/RoomHeaderAvatars.js Lines 26 to 37 in f0c6631 App/src/components/RoomHeaderAvatars.js Lines 55 to 61 in f0c6631 What changes do you think we should make in order to solve the problem?The first thing we should do is get personalDetails for each participants const participantProfiles = _.map(participants, participant => lodashGet(props.personalDetails, participant));we can pass participantProfiles to As suggested by @aimane-chnaif we should check if the displayName exists and show the name accordingly, the code below will behave like ProfilePage taking the details from Line 106 in 5f5dd02 or we can check if the displayName exists and if not use the details.login which is the email address or phone. const displayName = details.displayName ? details.displayName : details.login;<AttachmentModal
headerTitle={props.participantProfiles[0].login}
source={UserUtils.getFullSizeAvatar(props.icons[0].source, props.participantProfiles[index])}
isAuthTokenRequired
originalFileName={props.participantProfiles[index].originalFileName}
>we can also handle for multiple avatars <AttachmentModal
headerTitle={props.participantProfiles[index].login}
source={UserUtils.getFullSizeAvatar(props.icons[index].source, props.participantProfiles[index])}
isAuthTokenRequired
originalFileName={props.participantProfiles[index].originalFileName}
>We should also handle this every place Lines 143 to 164 in f0c6631 Result: Screen.Recording.2023-06-20.at.8.31.21.AM.movWhat alternative solutions did you explore? (Optional) |
|
@getusha Yeah if needed can be done in the same way. Just need to replicated the same code for each participants. |
|
Job added to Upwork: https://www.upwork.com/jobs/~015a875266a3d365a7 |
|
Current assignee @kevinksullivan is eligible for the External assigner, not assigning anyone new. |
|
Triggered auto assignment to Contributor-plus team member for initial proposal review - @aimane-chnaif ( |
|
@kevinksullivan, @aimane-chnaif Uh oh! This issue is overdue by 2 days. Don't forget to update your issues! |
|
Hi @aimane-chnaif can you review the proposals above? Thanks |
|
No satisfactory proposals yet |
|
@aimane-chnaif any comment on my proposal? Thanks |
|
@aimane-chnaif Can you please shed some light so I can improve my proposal. Because root cause is same that App/src/components/RoomHeaderAvatars.js Lines 26 to 37 in 1e308ca |
|
Let's make everything consistent between attachment preview modals diff.mov |
|
@aimane-chnaif Thanks. I'll update my proposal |
|
@aimane-chnaif I have updated my proposal Detail.page.preview.mp4 |
|
@aimane-chnaif what's the expected one? the email or the name? |
|
If user has display name set, should show display name. If not, email/phone. |
|
@aimane-chnaif thank you! for the clarification |
|
📣 @spcheema 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app! Offer link |
|
📣 @gadhiyamanan 🎉 An offer has been automatically sent to your Upwork account for the Reporter role 🎉 Thanks for contributing to the Expensify app! |
|
🎯 ⚡️ Woah @aimane-chnaif / @spcheema, great job pushing this forwards! ⚡️ The pull request got merged within 3 working days of assignment, so this job is eligible for a 50% #urgency bonus 🎉
On to the next one 🚀 |
|
@sonialiap I am back to handle payments when they come up! |
|
|
|
The solution for this issue has been 🚀 deployed to production 🚀 in version 1.3.55-8 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 2023-08-28. 🎊 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.
For reference, here are some details about the assignees on this issue:
As a reminder, here are the bonuses/penalties that should be applied for any External issue:
|
|
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:
|
|
Hey @aimane-chnaif please finish out the steps above so we can pay this out after the hold. Thanks! |
|
@aimane-chnaif please fill out the checklist so that we can issue payment |
|
This is feature request so I don't think regression test is needed |
|
Paid and contratcs closed! |
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:
should show preview of avatar
Actual Result:
does not show preview of avatar
Workaround:
Can the user still use Expensify without this being fixed? Have you informed them of the workaround?
Platforms:
Which of our officially supported platforms is this issue occurring on?
Version Number: 1.3.29-0
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: Any additional supporting documentation
Screen.Recording.2023-06-12.at.2.30.30.PM.mov
Recording.1026.mp4
Expensify/Expensify Issue URL:
Issue reported by: @gadhiyamanan
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1686560482456669
View all open jobs on GitHub
Upwork Automation - Do Not Edit
The text was updated successfully, but these errors were encountered: