-
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
Add ability to Take/Upload Receipts #23046
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
I'll send a PR today for that change 👍 |
🚀 Deployed to staging by https://github.com/luacmartins in version: 1.3.50-0 🚀
|
dragReceiptBeforeEmail: 'Drag a receipt onto this page, forward a receipt to ', | ||
dragReceiptAfterEmail: ' or choose a file to upload below.', | ||
chooseReceipt: 'Choose a receipt to upload or forward a receipt to ', | ||
chooseFile: 'Choose File', |
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.
chooseFile: 'Choose File', | |
chooseFile: 'Choose file', |
This is already in checklist:
- I verified any copy / text that was added to the app is grammatically correct in English. It adheres to proper capitalization guidelines (note: only the first word of header/labels should be capitalized), and is approved by marketing by adding the
Waiting for Copy
label for a copy review on the original GH to get the correct copy.
🚀 Deployed to production by https://github.com/puneetlath in version: 1.3.50-3 🚀
|
🚀 Deployed to staging by https://github.com/luacmartins in version: 1.3.51-0 🚀
|
<TopTab.Screen | ||
name={CONST.TAB.MANUAL} | ||
component={MoneyRequestAmount} | ||
initialParams={{reportID, iouType}} |
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.
Hi all, in this PR we broke the functionality where you set currency in the request param.
eg: /request/new?currency=CHF
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.
Oops, it looks like this is resolved now?
🚀 Deployed to production by https://github.com/Julesssss in version: 1.3.51-2 🚀
|
We added the |
|
||
const validateReceipt = (file) => { | ||
const {fileExtension} = FileUtils.splitExtensionFromFileName(lodashGet(file, 'name', '')); | ||
if (_.contains(CONST.API_ATTACHMENT_VALIDATIONS.UNALLOWED_EXTENSIONS, fileExtension.toLowerCase())) { |
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.
@AndrewGable Hi! While migrating this file to TS I noticed that CONST.API_ATTACHMENT_VALIDATIONS.UNALLOWED_EXTENSIONS
is used here, but it's not defined in CONST.ts
. Could you provide a list of unallowed extensions so I can add it to the migration PR?
const validateReceipt = (file) => { | ||
const {fileExtension} = FileUtils.splitExtensionFromFileName(lodashGet(file, 'name', '')); | ||
if (_.contains(CONST.API_ATTACHMENT_VALIDATIONS.UNALLOWED_EXTENSIONS, fileExtension.toLowerCase())) { | ||
Receipt.setUploadReceiptError(true, Localize.translateLocal('attachmentPicker.wrongFileType'), Localize.translateLocal('attachmentPicker.notAllowedExtension')); |
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.
It could have been better to define only key here and translate in render level so that it updates dynamically when user language changes on another device.
Issue: #25865
This case was missed while adding the ability to drag and drop while uploading Receipt. Issue: Desktop - Drag & drop container does not add opacity to entire height of the drawer Steps to reproduce:
|
// except ID is now required, and it gets a `selectedTab` from Onyx | ||
function OnyxTabNavigator({id, selectedTab, children, ...rest}) { | ||
return ( | ||
<TopTab.Navigator |
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.
We have encountered a minor issue in #26994. Without explicitly setting the keyboardDismissMode
, the navigator might interfere with the keyboard, resulting in unintended keyboard dismissal and input blurring particularly when switching tabs and interacting with input fields.
height={CONST.RECEIPT.ICON_SIZE} | ||
/> | ||
</View> | ||
<Text style={[styles.textReceiptUpload]}>{translate('receipt.upload')}</Text> |
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.
Coming from #27551
This was causing the tabs to swipe when selecting the text, since the cursor was outside the text area and it was triggering tab navigation (swipe gesture)
We resolved this by using panHandlers on the parent View (onPanResponderTerminationRequest: false
, don't allow something else (tab navigation in our case) to take over gestures) and extending text area to take full parent width
medium | ||
success | ||
text={translate('receipt.chooseFile')} | ||
style={[styles.p9]} |
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.
Basically, Button
is based on PressableWithFeedback
component, so we don't need to wrap it again here. It makes our custom style cause a bug here #26993
|
||
const camera = useRef(null); | ||
const [flash, setFlash] = useState(false); | ||
const [permissions, setPermissions] = useState('authorized'); |
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.
Coming from #26499, we switch to react-native-permissions
to check camera permissions.
Details
We're bringing Expensify's receipt scan feature to NewDot! Below is the feature spec:
Fixed Issues
$ https://github.com/Expensify/Expensify/issues/295238
Tests
Web/mWeb/Desktop
Test 1
Request Money
Manual
tab is selectedScan
tabChoose file
buttonRequest
buttonTest 2
Request Money
Scan
tabChoose file
buttonRequest
buttonTest 3
Request Money
Scan
tabRequest
buttonTest 4, 5, 6
Android/iOS
Test 7
Request Money
Manual
tab is selectedScan
tabRequest
buttonTest 8
Request Money
Scan
tabRequest
buttonTest 9
Request Money
Scan
tabRequest
buttonTest 10, 11, 12
Offline tests
All tests should work offline, however, we are going to be saving the images to disk in a future follow up PR.
QA Steps
Tests 1-12 above should be run by QA
PR Author Checklist
### Fixed Issues
section aboveTests
sectionOffline steps
sectionQA steps
sectiontoggleReport
and notonIconClick
)myBool && <MyComponent />
.src/languages/*
files and using the translation methodWaiting for Copy
label for a copy review on the original GH to get the correct copy.STYLE.md
) were followedAvatar
, I verified the components usingAvatar
are working as expected)/** comment above it */
this
properly so there are no scoping issues (i.e. foronClick={this.submit}
the methodthis.submit
should be bound tothis
in the constructor)this
are necessary to be bound (i.e. avoidthis.submit = this.submit.bind(this);
ifthis.submit
is never passed to a component event handler likeonClick
)StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG)
)Avatar
is modified, I verified thatAvatar
is working as expected in all cases)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
Web
Screen.Recording.2023-07-21.at.2.28.37.PM.mov
Mobile Web - Chrome
Mobile Web - Safari
Desktop
iOS
Android