-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
[$500] Android- Attachment - Image clicked from Whatsapp zoomed too much in preview #26086
Comments
Triggered auto assignment to @JmillsExpensify ( |
Bug0 Triage Checklist (Main S/O)
|
ProposalPlease re-state the problem that we are trying to solve in this issue.Android- Attachment - Image clicked from Whatsapp zoomed too much in preview What is the root cause of that problem?The issue is predominantly observed with photos taken using the WhatsApp camera on Android when rendered in the React Native app. This suggests that there might be unique attributes or peculiarities associated with these images, such as their format, metadata, or encoding differences. Additionally, given the platform specificity of the problem, there are likely platform-specific behaviors or quirks in React Native's image handling on Android that exacerbate the issue, especially when combined with the nuances of images from the WhatsApp camera. What changes do you think we should make in order to solve the problem?Option 1To address the unusual rendering behavior observed with images taken from the WhatsApp camera on Android, I recommend employing a dynamic resizing strategy based on the image's load state.
The solution involves setting the resizeMode property of the React Native Image component as: This approach ensures that once the image has finished loading , it will display using the contain mode. The contain mode scales the image to fit within its container dimensions, guaranteeing the entire image is visible without distortion or cropping. Given that the container's dimensions are already set, leveraging contain will yield consistent and predictable rendering across different images and scenarios. In contrast, during the loading phase or if the image isn't cached, the cover mode is employed to fill the container's bounds, which can be a suitable placeholder behavior. Option 2We can also create a state variable to track if the image size is calculated or not and based on that we can set Resultfixed_demo.mp4 |
@JmillsExpensify Uh oh! This issue is overdue by 2 days. Don't forget to update your issues! |
I can reproduce with these images also, these are clicked from Whatsapp but downloaded from Expensify. https://github.com/Expensify/App/assets/85894871/2a4678d0-80fe-45bd-87d3-c1af570d1ac6 |
@JmillsExpensify 6 days overdue. This is scarier than being forced to listen to Vogon poetry! |
Hmm, I agree that we should treat copied images, and just as importantly, we should show cropped images equivalently. Though for clarity, is this only the case for WhatsApp images or can you reproduce more generally. If WhatsApp only, I don't think that counts as a bug, as it's just one app (sure a very popular one). If this is possible more generally for all images, I think we should address |
@JmillsExpensify, I will try to reproduce it in other cases as well but IMO this should be valid bug because:
For testing you can download these images, it is downloaded from Expensify. Pls check my proposal result demo. |
@JmillsExpensify this issue was created 2 weeks ago. Are we close to a solution? Let's make sure we're treating this as a top priority. Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks! |
@JmillsExpensify this issue was created 2 weeks ago. Are we close to a solution? Let's make sure we're treating this as a top priority. Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks! |
@JmillsExpensify Eep! 4 days overdue now. Issues have feelings too... |
I'll add the external label to see what C+ thinks about this report. |
Job added to Upwork: https://www.upwork.com/jobs/~0170c628d9805f4ad5 |
Current assignee @JmillsExpensify is eligible for the External assigner, not assigning anyone new. |
Triggered auto assignment to Contributor-plus team member for initial proposal review - @situchan ( |
Do we know which agency is interested in picking this up? |
@JmillsExpensify, @situchan Uh oh! This issue is overdue by 2 days. Don't forget to update your issues! |
Maybe callstack |
@situchan, Could you please take a look at this issue? It has been open since 2016 in react-native repo, and there is only a workaround that works, very similar to our issue. I believe we can add the workaround I suggested, as it is a very minor change that only adds a key prop and triggers a re-render once we change the width and height. |
If it's old RN issue, I was fine to apply workaround. But this obviously came from react-native-fast-image |
I'll check again. I've seen in react-native-fast-image GH issue that they mentioned it as an upstream issue in react-native. |
Still discussing |
Same |
Same |
@JmillsExpensify @situchan, we can assign someone else from callstack if we don't want to add a key prop as suggested in my proposal because I have spent enough time on this and couldn't find any better solution. |
DylanVann/react-native-fast-image#762, this is a similar issue related to re-rendering and only solution they found is to switch to native images or add a key prop. |
@Krishna2323 we're no longer using react-native-fast-image but expo-image |
Maybe, then, the issue should be resolved already. will check |
@Krishna2323 did you check already? |
@situchan, yes the issue has been resolved. |
ok so it's 100% issue from react-native-fast-image package. @JmillsExpensify let's close |
If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!
Action Performed:
Expected Result:
The image preview should be displayed properly.
Actual Result:
The image preview is zoomed too much.
Workaround:
Unknown
Platforms:
Which of our officially supported platforms is this issue occurring on?
Version Number: 1.3.57.5
Reproducible in staging?: n/a
Reproducible in production?: n/a
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
WhatsApp.Video.2023-08-19.at.18.35.52.1.mp4
Expensify/Expensify Issue URL:
Issue reported by: krishna2323
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1692464365619889
View all open jobs on GitHub
Upwork Automation - Do Not Edit
The text was updated successfully, but these errors were encountered: