-
Notifications
You must be signed in to change notification settings - Fork 3k
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 2024-11-14] [$500] Android - Avatar - Profile avatar can be seen only if tap on blank screen #49553
Comments
Triggered auto assignment to @puneetlath ( |
@puneetlath FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors |
Job added to Upwork: https://www.upwork.com/jobs/~021837227522028493087 |
Triggered auto assignment to Contributor-plus team member for initial proposal review - @dukenv0307 ( |
Edited by proposal-police: This proposal was edited at 2024-09-21 10:37:45 UTC.``` ProposalPlease re-state the problem that we are trying to solve in this issue.Android - Avatar - Profile avatar can be seen only if tap on blank screen What is the root cause of that problem?The layout is not changing, so App/src/components/Lightbox/index.tsx Lines 204 to 207 in 9160fa5
So, the App/src/components/Lightbox/index.tsx Lines 94 to 95 in 9160fa5
App/src/components/Lightbox/index.tsx Line 208 in 9160fa5
The reason What changes do you think we should make in order to solve the problem?To solve this problem, we just need to render the // .src/components/AttachmentModal.tsx#L580
- {!!onConfirm && !isConfirmButtonDisabled && (
+ {!!onConfirm && !isConfirmButtonDisabled && shouldLoadAttachment && (
<Button
ref={viewRef(submitRef)}
success
large
style={[styles.buttonConfirm, shouldUseNarrowLayout ? {} : styles.attachmentButtonBigScreen]}
textStyles={[styles.buttonConfirmText]}
text={translate('common.send')}
onPress={submitAndClose}
isDisabled={isConfirmButtonDisabled}
pressOnEnter
/>
)} Note Note: As I tested, this issue only occurs on Android. We can either update it for Android only or for all platforms, and it will still work under this condition What alternative solutions did you explore? (Optional) Alternative Solutions 1
// .src/components/AttachmentModal.tsx#L368
} else if (fileObject.uri) {
const inputModalType = getModalType(fileObject.uri, fileObject);
+ setShouldLoadAttachment(true);
setIsModalOpen(true);
setSourceState(fileObject.uri);
setFile(fileObject);
setModalType(inputModalType);
} Alternative Solutions 2We should use a ref so that the measurement occurs after the component has mounted, something like this: // .src/components/Lightbox/index.tsx#L167
+ const viewRef = useRef<View>(null);
// .src/components/Lightbox/index.tsx#L172
+ useEffect(() => {
+ const measureView = () => {
+ viewRef.current?.measure((_, __, width, height) => {
+ setCanvasSize({
+ width: PixelRatio.roundToNearestPixel(width),
+ height: PixelRatio.roundToNearestPixel(height),
+ });
+ });
+ };
+ measureView();
+ }, []);
// .src/components/Lightbox/index.tsx#L204
<View
style={[StyleSheet.absoluteFill, style]}
onLayout={updateCanvasSize}
+ ref={viewRef}
>
POC
Screen_Recording_20240921_173421_Camera.mp4 |
ProposalPlease re-state the problem that we are trying to solve in this issue.Avatar image does not show up or load. What is the root cause of that problem?On mobile, we use Lightbox to render the avatar image, however Which also means the image view is never mounted and App/src/components/Lightbox/index.tsx Lines 204 to 207 in 9160fa5
What changes do you think we should make in order to solve the problem?Initialize canvasSize with any value (i.e 0x0) will force a layout and mount the image view and thus load the image. so we replace App/src/components/Lightbox/index.tsx Line 94 in 9160fa5
with const [canvasSize, setCanvasSize] = useState<CanvasSize>({width: 0, height: 0}); What alternative solutions did you explore? (Optional)N/A avatar.mp4 |
@huult @struc Thanks for your proposals. FYI, I just tested
|
Updated Proposal
|
@huult Can you elaborate that? |
Updated Proposal
Apologies, @dukenv0307 for the many changes in the proposal. Could you please review it again? I've updated the problem with more accuracy, and I appreciate your help. Thank you! |
@huult It doesn't really convince me, why does this happen on Android only? |
📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸 |
@dukenv0307 , With the solution I provided, rendering the // .src/components/AttachmentModal.tsx#L580
- {!!onConfirm && !isConfirmButtonDisabled && (
+ {!!onConfirm && !isConfirmButtonDisabled && shouldLoadAttachment && ( |
@puneetlath, @dukenv0307 Eep! 4 days overdue now. Issues have feelings too... |
@puneetlath, @dukenv0307 Still overdue 6 days?! Let's take care of this! |
I tested this issue on React Native expo, I didn't face this issue. Can you dig deeper to find the exact RCA? We also open to receive more proposals |
📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸 |
@puneetlath @dukenv0307 this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks! |
@huult are you still interested in trying to find a solution for this? |
@puneetlath , Currently, I have found a workaround solution to fix this issue by rendering the With the change below, I tested it and it's working as expected. // .src/components/AttachmentModal.tsx#L580
- {!!onConfirm && !isConfirmButtonDisabled && (
+ {!!onConfirm && !isConfirmButtonDisabled && shouldLoadAttachment && ( |
Triggered auto assignment to @danieldoglas, see https://stackoverflow.com/c/expensify/questions/7972 for more details. |
@danieldoglas Can you check this issue? Thanks |
📣 @dukenv0307 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app! |
Assigned @s77rt ! |
|
The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.58-2 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 2024-11-14. 🎊 For reference, here are some details about the assignees on this issue:
|
@s77rt / @dukenv0307 @puneetlath The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed. Please copy/paste the BugZero Checklist from here into a new comment on this GH and complete it. If you have the K2 extension, you can simply click: [this button] |
BugZero Checklist:
Bug classificationSource of bug:
Where bug was reported:
Who reported the bug:
Regression Test Proposal Template
Regression Test ProposalPrecondition:Test:Do we agree 👍 or 👎 |
Payment summary:
@dukenv0307 can you please accept the Upwork offer and ping me here when you have? |
@puneetlath Accepted, thank you! |
$500 approved for @s77rt |
All paid. Thanks y'all! |
If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!
Version Number: 9.0.39-0
Reproducible in staging?: Y
Reproducible in production?: Y
Issue reported by: Applause Internal Team
Action Performed:
Expected Result:
Profile avatar should be displayed when open it
Actual Result:
Profile avatar can be seen only if tap on blank screen
Same issue occurs when Take photo
Workaround:
Unknown
Platforms:
Screenshots/Videos
Bug6610027_1726846111222.Recording__4007.mp4
View all open jobs on GitHub
Upwork Automation - Do Not Edit
Issue Owner
Current Issue Owner: @s77rtThe text was updated successfully, but these errors were encountered: