-
Notifications
You must be signed in to change notification settings - Fork 3.2k
[Pre RN 0.79] Migrate documents picker #59363
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
[Pre RN 0.79] Migrate documents picker #59363
Conversation
|
This comment was marked as duplicate.
This comment was marked as duplicate.
This comment was marked as duplicate.
This comment was marked as duplicate.
This comment was marked as duplicate.
This comment was marked as duplicate.
# Conflicts: # src/components/AttachmentPicker/index.native.tsx
This comment was marked as duplicate.
This comment was marked as duplicate.
# Conflicts: # src/components/AttachmentPicker/index.native.tsx
@AndrewGable Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
🚧 @mountiny has triggered a test app build. You can view the workflow run here. |
This comment has been minimized.
This comment has been minimized.
@mountiny This can't be tested on Hybrid iOS App on simulator. |
![]() @war-in When picking the PDF file, this error appears. |
@shubham1206agra I tested on my iOS device and there is no error on this PR but asked Vit for a retest |
Will test later thanks |
🚧 @mountiny has triggered a test app build. You can view the workflow run here. |
🧪🧪 Use the links below to test this adhoc build on Android, iOS, Desktop, and Web. Happy testing! 🧪🧪 |
Retesting the issue in question: https://expensify.slack.com/archives/C049HHMV9SM/p1744647887778349 |
yeah, so on the latest staging TestFlight version, I can upload an heic and view it just fine. But on this AdHoc build I could upload it fine but then not view it afterwards. So I think this PR would cause a regression unless that's fixed |
It worked |
I mean uploading heic photo from the gallery on iPhone |
Well I was able to upload the photo (and even view it on other devices), but on the AdHoc build I couldn't view it on the device in which it was uploaded. The preview showed, but after upload it was not rendering at all. Not sure if that will be reproducible for others as well |
@roryabraham yeah, facing the same issue, but! The bug might be caused by the new reports project, which is currently behind beta (and we all have access to it). I logged into my regular gmail account and the flow works fine so the problem seems unrelated :) |
If its related to |
@war-in Cna you please update the test/ QA steps to include the ios flow scanning a receipt for expense and choosing an heic image from gallery? @shubham1206agra Can you please complete the review/ approve if all works well for you? thanks! |
@mountiny steps updated |
@shubham1206agra were you able to reproduce the issue Rory mentioned? |
@allgandalf tested this here https://expensify.slack.com/archives/C02NK2DQWUX/p1744895681338739?thread_ts=1744893413.115159&cid=C02NK2DQWUX and he did not encountered this issue. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have tested again on the adhoc build and both creating expense and adding attachment worked fine for me also I could see the images just fine on the build and on staging web after ✅
I think we are ready to go and test this out so going to merge this and the OldApp PR |
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
🚀 Deployed to staging by https://github.com/mountiny in version: 9.1.31-0 🚀
|
🚀 Deployed to production by https://github.com/AndrewGable in version: 9.1.31-3 🚀
|
@@ -100,6 +100,7 @@ | |||
"@react-native-clipboard/clipboard": "^1.15.0", | |||
"@react-native-community/geolocation": "3.3.0", | |||
"@react-native-community/netinfo": "11.2.1", | |||
"@react-native-documents/picker": "^10.1.1", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Upgrading @react-native-documents/picker
to 10.1.1
caused this bug: #60600
Explanation of Change
I'm working on RN 0.78 support #58914 and I found out that the
@react-native-document-picker
has to be bumped because it supports React Native 0.78 since 10.x.xFixed Issues
$ #57511
PROPOSAL:
MOBILE-EXPENSIFY: https://github.com/Expensify/Mobile-Expensify/pull/13510
Tests
Open app and verify it runs correctly
Go to chat and send attachment
Verify that the flow is correct
In ios native, select Create Expense
Scan and choose image taken on the iphone in gallery (so its the ios heic format)
Note you have to choose the image from gallery, taking the picture in the app works fine
Create this expense
Verify it works
Offline tests
QA Steps
Open app and verify it runs correctly
Go to chat and send attachment
Verify that the flow is correct
In ios native, select Create Expense
Scan and choose image taken on the iphone in gallery (so its the ios heic format)
Note you have to choose the image from gallery, taking the picture in the app works fine
Create this expense
Verify it works
PR Author Checklist
### Fixed Issues
section aboveTests
sectionOffline steps
sectionQA steps
sectiontoggleReport
and notonIconClick
)src/languages/*
files and using the translation methodSTYLE.md
) were followedAvatar
, I verified the components usingAvatar
are working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG)
)Avatar
is modified, I verified thatAvatar
is working as expected in all cases)Design
label and/or tagged@Expensify/design
so the design team can review the changes.ScrollView
component to make it scrollable when more elements are added to the page.main
branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTest
steps.Screenshots/Videos
Android: Native
Screen.Recording.Apr.11.2025.from.Marcin.Warchol.mp4
Android: mWeb Chrome
iOS: Native
O.Rany.Messenger.Video.mp4
iOS: mWeb Safari
MacOS: Chrome / Safari
MacOS: Desktop