feat: Change the old file picker with new native capacitor plugin #3846
Conversation
amit429
requested review from
Chethan-Fyle,
arjunaj5 and
mvaishnavi
as code owners
WalkthroughListen up, boss! The code now swaps out the old file upload logic for a new, stylish attachment flow using Capacitor plugins. The add-edit-expense page gets a refactor: camera and gallery options pop up, old file input tricks are gone, and HEIC images get special treatment. Supporting dependencies are added, and the camera options popup component is trimmed down to the essentials. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant AddEditExpensePage
participant CameraOptionsPopupComponent
participant CapacitorCamera
participant CapacitorFilePicker
participant Toast
User->>AddEditExpensePage: Clicks "Add Attachment"
AddEditExpensePage->>CameraOptionsPopupComponent: Opens popover
User->>CameraOptionsPopupComponent: Selects "Camera" or "Gallery"
CameraOptionsPopupComponent->>AddEditExpensePage: Returns option
alt Camera selected
AddEditExpensePage->>CapacitorCamera: openCamera()
CapacitorCamera-->>AddEditExpensePage: Photo DataUrl
AddEditExpensePage->>Toast: Show success toast
else Gallery selected
AddEditExpensePage->>CapacitorFilePicker: openGallery()
CapacitorFilePicker-->>AddEditExpensePage: File(s) Data
alt HEIC file
AddEditExpensePage->>CapacitorFilePicker: Convert HEIC to JPEG
CapacitorFilePicker-->>AddEditExpensePage: JPEG Data
end
AddEditExpensePage->>Toast: Show success toast
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15 minutes Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
🔁 Code Duplication Report - Angular
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🔭 Outside diff range comments (1)
src/app/fyle/add-edit-expense/camera-options-popup/camera-options-popup.component.ts (1)
19-21: Empty ngOnInit? Even I don't enter the scene without a purpose!This lifecycle hook is doing nothing, partner. If you don't need initialization logic, remove it completely.
- ngOnInit(): void { - return; - }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (3)
package.json(2 hunks)src/app/fyle/add-edit-expense/add-edit-expense.page.ts(2 hunks)src/app/fyle/add-edit-expense/camera-options-popup/camera-options-popup.component.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.component.{ts,html}
📄 CodeRabbit Inference Engine (.cursor/rules/component-i18n-key-naming.mdc)
**/*.component.{ts,html}: Top-level object in i18n translation files must be the component or feature folder name, derived from the file name by converting kebab-case to camelCase and removing the first matching prefix from: 'feature', 'ui', 'component'.
Keys inside the top-level object in i18n translation files must be free-form, not prefixed, and there must be no nested objects beyond the first level.
Use meaningful, semantic, and context-aware key names for i18n keys; avoid generic names like 'label1', 'text1', or 'message1'.
Do not translate strings that are already translation keys, tracking/analytics strings, strings with only special characters, comments, non-user-visible text, or variable bindings and expressions.
One object per component or feature in i18n translation files; avoid deep nesting and avoid prefixes like 'ui*' or 'feature*' in object names.
Files:
src/app/fyle/add-edit-expense/camera-options-popup/camera-options-popup.component.ts
src/**/*.component.{ts,html}
📄 CodeRabbit Inference Engine (.cursor/rules/component-i18n-key-naming.mdc)
Translation keys for Angular components and templates located in 'src/**' must be placed in 'src/assets/i18n/{lang}.json' for every supported language.
Files:
src/app/fyle/add-edit-expense/camera-options-popup/camera-options-popup.component.ts
🧠 Learnings (15)
📓 Common learnings
Learnt from: bistaastha
PR: fylein/fyle-mobile-app#3423
File: src/app/fyle/spender-onboarding/spender-onboarding.page.html:0-0
Timestamp: 2025-01-20T02:26:35.101Z
Learning: In the Fyle mobile app's spender onboarding flow, the team prefers to keep the loader implementation within the spender-onboarding.page.html component rather than extracting it into a separate reusable component.
Learnt from: sumrender
PR: fylein/fyle-mobile-app#3265
File: src/app/fyle/personal-cards-matched-expenses/expense-preview/expense-preview.component.spec.ts:35-37
Timestamp: 2024-11-18T06:55:17.972Z
Learning: In `src/app/fyle/add-edit-expense/add-edit-expense.page.ts`, the `unmatchExpense` method is used for corporate card expenses, not for personal card expenses.
📚 Learning: in `src/app/shared/components/fy-loading-screen/fy-loading-screen.component.html`, the conditional s...
Learnt from: SahilK-027
PR: fylein/fyle-mobile-app#3312
File: src/app/shared/components/fy-loading-screen/fy-loading-screen.component.html:19-22
Timestamp: 2024-12-09T07:08:28.540Z
Learning: In `src/app/shared/components/fy-loading-screen/fy-loading-screen.component.html`, the conditional size logic for `[attr.size]` will be removed after the new design changes are applied across the entire mobile app, so refactoring is unnecessary at this time.
Applied to files:
src/app/fyle/add-edit-expense/camera-options-popup/camera-options-popup.component.ts
📚 Learning: in 'src/app/fyle/manage-corporate-cards/manage-corporate-cards.page.spec.ts', the flags 'isvisartfen...
Learnt from: bistaastha
PR: fylein/fyle-mobile-app#3240
File: src/app/fyle/manage-corporate-cards/manage-corporate-cards.page.spec.ts:293-354
Timestamp: 2024-10-23T11:45:16.974Z
Learning: In 'src/app/fyle/manage-corporate-cards/manage-corporate-cards.page.spec.ts', the flags 'isVisaRTFEnabled$', 'isMastercardRTFEnabled$', and 'isYodleeEnabled$' do not have rapidly changing values, so tests for rapid changes in these flags are unnecessary.
Applied to files:
src/app/fyle/add-edit-expense/camera-options-popup/camera-options-popup.component.tssrc/app/fyle/add-edit-expense/add-edit-expense.page.ts
📚 Learning: in `src/app/fyle/add-edit-expense/add-edit-expense.page.ts`, the `unmatchexpense` method is used for...
Learnt from: sumrender
PR: fylein/fyle-mobile-app#3265
File: src/app/fyle/personal-cards-matched-expenses/expense-preview/expense-preview.component.spec.ts:35-37
Timestamp: 2024-11-18T06:55:17.972Z
Learning: In `src/app/fyle/add-edit-expense/add-edit-expense.page.ts`, the `unmatchExpense` method is used for corporate card expenses, not for personal card expenses.
Applied to files:
src/app/fyle/add-edit-expense/camera-options-popup/camera-options-popup.component.tssrc/app/fyle/add-edit-expense/add-edit-expense.page.ts
📚 Learning: in the 'fyle-mobile-app' codebase, adding 'as const' to formgroup control definitions is not necessa...
Learnt from: bistaastha
PR: fylein/fyle-mobile-app#3289
File: src/app/post-verification/invited-user/invited-user.page.ts:69-70
Timestamp: 2024-12-04T05:23:28.646Z
Learning: In the 'fyle-mobile-app' codebase, adding 'as const' to FormGroup control definitions is not necessary.
Applied to files:
src/app/fyle/add-edit-expense/camera-options-popup/camera-options-popup.component.tssrc/app/fyle/add-edit-expense/add-edit-expense.page.ts
📚 Learning: in `src/app/fyle/my-expenses/my-expenses.page.spec.ts`, when testing private methods like `filterdei...
Learnt from: arjunaj5
PR: fylein/fyle-mobile-app#3242
File: src/app/fyle/my-expenses/my-expenses.page.spec.ts:967-968
Timestamp: 2024-11-04T13:10:36.868Z
Learning: In `src/app/fyle/my-expenses/my-expenses.page.spec.ts`, when testing private methods like `filterDEIncompleteExpenses`, it's acceptable to use `//@ts-ignore` to spy on them in unit tests.
Applied to files:
src/app/fyle/add-edit-expense/camera-options-popup/camera-options-popup.component.tssrc/app/fyle/add-edit-expense/add-edit-expense.page.ts
📚 Learning: in 'src/app/post-verification/invited-user/invited-user.page.ts', the 'password' and 'fullname' obta...
Learnt from: bistaastha
PR: fylein/fyle-mobile-app#3289
File: src/app/post-verification/invited-user/invited-user.page.ts:97-98
Timestamp: 2024-12-04T05:23:21.200Z
Learning: In 'src/app/post-verification/invited-user/invited-user.page.ts', the 'password' and 'fullName' obtained from 'this.fg.controls.password.value' and 'this.fg.controls.fullName.value' will always be strings, so additional error handling for type checking is unnecessary.
Applied to files:
src/app/fyle/add-edit-expense/camera-options-popup/camera-options-popup.component.tssrc/app/fyle/add-edit-expense/add-edit-expense.page.ts
📚 Learning: in the angular application, the `aria-expanded` attribute is not needed in `password-check-tooltip.c...
Learnt from: bistaastha
PR: fylein/fyle-mobile-app#3281
File: src/app/shared/components/password-check-tooltip/password-check-tooltip.component.html:0-0
Timestamp: 2024-12-03T05:20:24.662Z
Learning: In the Angular application, the `aria-expanded` attribute is not needed in `password-check-tooltip.component.html`.
Applied to files:
src/app/fyle/add-edit-expense/camera-options-popup/camera-options-popup.component.ts
📚 Learning: in this codebase, properties used in angular templates are not explicitly declared as 'public', sinc...
Learnt from: bistaastha
PR: fylein/fyle-mobile-app#3240
File: src/app/fyle/manage-corporate-cards/manage-corporate-cards.page.html:14-14
Timestamp: 2024-10-23T09:23:56.684Z
Learning: In this codebase, properties used in Angular templates are not explicitly declared as 'public', since they are public by default in TypeScript.
Applied to files:
src/app/fyle/add-edit-expense/camera-options-popup/camera-options-popup.component.tssrc/app/fyle/add-edit-expense/add-edit-expense.page.ts
📚 Learning: in the angular application at `src/app/shared/components/password-check-tooltip/password-check-toolt...
Learnt from: bistaastha
PR: fylein/fyle-mobile-app#3281
File: src/app/shared/components/password-check-tooltip/password-check-tooltip.component.scss:1-77
Timestamp: 2024-11-26T14:36:10.771Z
Learning: In the Angular application at `src/app/shared/components/password-check-tooltip/password-check-tooltip.component.scss`, dark mode is not supported, so styles and media queries for dark mode should not be added.
Applied to files:
src/app/fyle/add-edit-expense/camera-options-popup/camera-options-popup.component.ts
📚 Learning: in angular code, prefer using the `tap` operator to set state variables within observable streams fo...
Learnt from: bistaastha
PR: fylein/fyle-mobile-app#3306
File: src/app/auth/pending-verification/pending-verification.page.ts:45-45
Timestamp: 2024-12-06T10:17:48.921Z
Learning: In Angular code, prefer using the `tap` operator to set state variables within observable streams for better readability, as seen in `pending-verification.page.ts`.
Applied to files:
src/app/fyle/add-edit-expense/camera-options-popup/camera-options-popup.component.tssrc/app/fyle/add-edit-expense/add-edit-expense.page.ts
📚 Learning: applies to **/*.component.{ts,html} : one object per component or feature in i18n translation files;...
Learnt from: CR
PR: fylein/fyle-mobile-app#0
File: .cursor/rules/component-i18n-key-naming.mdc:0-0
Timestamp: 2025-07-28T05:57:57.796Z
Learning: Applies to **/*.component.{ts,html} : One object per component or feature in i18n translation files; avoid deep nesting and avoid prefixes like 'ui*' or 'feature*' in object names.
Applied to files:
src/app/fyle/add-edit-expense/camera-options-popup/camera-options-popup.component.tssrc/app/fyle/add-edit-expense/add-edit-expense.page.ts
📚 Learning: applies to **/*.component.{ts,html} : top-level object in i18n translation files must be the compone...
Learnt from: CR
PR: fylein/fyle-mobile-app#0
File: .cursor/rules/component-i18n-key-naming.mdc:0-0
Timestamp: 2025-07-28T05:57:57.796Z
Learning: Applies to **/*.component.{ts,html} : Top-level object in i18n translation files must be the component or feature folder name, derived from the file name by converting kebab-case to camelCase and removing the first matching prefix from: 'feature', 'ui', 'component'.
Applied to files:
src/app/fyle/add-edit-expense/camera-options-popup/camera-options-popup.component.tssrc/app/fyle/add-edit-expense/add-edit-expense.page.ts
📚 Learning: applies to **/*.{service,pipe,directive}.ts : convert kebab-case file names to camelcase and remove ...
Learnt from: CR
PR: fylein/fyle-mobile-app#0
File: .cursor/rules/atomics-i18n-key-pattern.mdc:0-0
Timestamp: 2025-07-28T05:57:50.701Z
Learning: Applies to **/*.{service,pipe,directive}.ts : Convert kebab-case file names to camelCase and remove the suffix (Service, Pipe, Directive) when generating i18n keys for services, pipes, and directives
Applied to files:
src/app/fyle/add-edit-expense/add-edit-expense.page.ts
📚 Learning: in `addeditexpensepage`, the `vendor` parameter passed to `filtervendor` is always non-null because ...
Learnt from: harshal015
PR: fylein/fyle-mobile-app#3214
File: src/app/fyle/add-edit-expense/add-edit-expense.page.ts:4558-4560
Timestamp: 2024-09-26T20:56:20.568Z
Learning: In `AddEditExpensePage`, the `vendor` parameter passed to `filterVendor` is always non-null because it is validated before calling the method.
Applied to files:
src/app/fyle/add-edit-expense/add-edit-expense.page.ts
🧬 Code Graph Analysis (1)
src/app/fyle/add-edit-expense/add-edit-expense.page.ts (1)
src/app/core/constants.ts (1)
MAX_FILE_SIZE(1-1)
🔇 Additional comments (8)
package.json (1)
53-53: Style! The new Capacitor dependencies are perfectly aligned.Mind it! These dependencies @capacitor/filesystem (^7.1.4) and @capawesome/capacitor-file-picker (^7.2.0) are in perfect sync with your existing Capacitor ecosystem. When the superstar enters, everything falls into place!
Also applies to: 63-63
src/app/fyle/add-edit-expense/camera-options-popup/camera-options-popup.component.ts (2)
1-1: Kabali da! Clean imports, clean code.The import statement is now lean and mean, just like how I like my punch dialogues - straight to the point!
33-37: Simplified like my one-liner! Perfect execution.The method now focuses on its core duty - track the action and dismiss with style. No unnecessary complications, just pure action!
src/app/fyle/add-edit-expense/add-edit-expense.page.ts (5)
5-7: Thalaivaa says: These imports are power-packed like my punch dialogues!The new Capacitor plugin imports look good. They're properly organized and will handle the native file operations with style.
4850-4854: Style-a irukku! File size validation - perfect like my timing!Excellent job checking the file size before processing. The MAX_FILE_SIZE constant is properly imported and used.
4859-4867: Magizhchi! HEIC to JPEG conversion - modern problems need modern solutions!Great implementation of HEIC conversion for iOS compatibility. The conversion is handled smoothly with proper path management.
4887-4893: Simple but powerful method - like my one-liners!The
showAddedToExpenseToastmethod efficiently handles success notifications and resets the error state. Well done!
4843-4846: 🔥 Confirm Supported Receipt Formats, Superstar!
En vazhi, thani vazhi! Our picker currently accepts JPEG, PNG, PDF, and HEIC. In enterprise expense systems, the most common and recommended formats are PDF, JPEG/JPG, and PNG—TIFF is occasionally used for high-quality scans, while HEIC isn’t widely supported. Please verify against your product requirements:• File: src/app/fyle/add-edit-expense/add-edit-expense.page.ts (Lines 4843–4846)
– Revisit thetypesarray inFilePicker.pickFiles.
– Remove'image/heic'if HEIC isn’t officially supported.
– Consider adding'image/tiff'only if your users need high-quality scanned receipts.Keep the experience superpowered and lean, my friend!
| async addAttachments(event: Event): Promise<void> { | ||
| event.stopPropagation(); | ||
| const popup = await this.popoverController.create({ | ||
| component: CameraOptionsPopupComponent, | ||
| cssClass: 'camera-options-popover', | ||
| }); | ||
|
|
||
| await popup.present(); | ||
|
|
||
| const { data } = (await popup.onWillDismiss()) as { | ||
| data: { | ||
| option?: string; | ||
| type: string; | ||
| dataUrl: string; | ||
| actionSource?: string; | ||
| }; | ||
| }; | ||
|
|
||
| if (data && data.option) { | ||
| if (data.option === 'camera') { | ||
| this.openCamera(); | ||
| } else { | ||
| this.showSizeLimitExceededPopover(MAX_FILE_SIZE); | ||
| this.openGallery(); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Mind it! This popup flow is smooth as my stunts, but needs one small touch!
The addAttachments method is well-structured. However, when the popup is dismissed without selecting an option, the data could be undefined. Consider adding a null check.
- if (data && data.option) {
+ if (data?.option) {
if (data.option === 'camera') {
this.openCamera();
} else {
this.openGallery();
}
}🤖 Prompt for AI Agents
In src/app/fyle/add-edit-expense/add-edit-expense.page.ts around lines 4790 to
4815, the addAttachments method assumes data is always defined after the popup
dismissal, but data can be undefined if the popup is dismissed without
selection. Add a null or undefined check for data before accessing its
properties to prevent runtime errors. Adjust the condition to safely handle
cases when data is missing.
| async openCamera(): Promise<void> { | ||
| try { | ||
| const image = await Camera.getPhoto({ | ||
| quality: 90, | ||
| allowEditing: false, | ||
| resultType: CameraResultType.DataUrl, | ||
| source: CameraSource.Camera, | ||
| }); | ||
|
|
||
| async addAttachments(event: Event): Promise<void> { | ||
| event.stopPropagation(); | ||
| if (image && image.dataUrl) { | ||
| const receiptDetails = { | ||
| type: this.fileService.getImageTypeFromDataUrl(image.dataUrl), | ||
| dataUrl: image.dataUrl, | ||
| actionSource: 'camera', | ||
| }; | ||
| this.attachReceipts(receiptDetails); | ||
| this.showAddedToExpenseToast(); | ||
| } | ||
| // eslint-disable-next-line @typescript-eslint/no-unused-vars | ||
| } catch (error) { | ||
| this.showSnackBarToast({ message: 'Error opening camera' }, 'failure', ['msb-error-with-camera-icon']); | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Kabali da! Camera handling is super, but error tracking needs more power!
The camera error handling catches the error but doesn't log it. In production, you'll want to know what went wrong.
} catch (error) {
+ console.error('Camera error:', error);
+ this.trackingService.captureException(error);
this.showSnackBarToast({ message: 'Error opening camera' }, 'failure', ['msb-error-with-camera-icon']);
}🤖 Prompt for AI Agents
In src/app/fyle/add-edit-expense/add-edit-expense.page.ts around lines 4817 to
4840, the catch block for the openCamera method handles errors by showing a
toast but does not log the actual error details. To fix this, add a logging
statement inside the catch block that records the caught error, so you can track
and debug camera-related issues effectively in production.
| } catch (error) { | ||
| this.showSnackBarToast({ message: 'Error opening gallery' }, 'failure', ['msb-error-with-camera-icon']); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Gallery error also needs the same punch as camera error, machi!
Similar to the camera error, the gallery error should also be logged for debugging.
} catch (error) {
+ console.error('Gallery error:', error);
+ this.trackingService.captureException(error);
this.showSnackBarToast({ message: 'Error opening gallery' }, 'failure', ['msb-error-with-camera-icon']);
}🤖 Prompt for AI Agents
In src/app/fyle/add-edit-expense/add-edit-expense.page.ts around lines 4882 to
4883, the catch block for gallery errors only shows a toast message but does not
log the error. Add a logging statement to capture and log the error details for
debugging, similar to how camera errors are logged, before showing the toast
notification.
1 participant
Clickup
https://app.clickup.com/
Code Coverage
Please add code coverage here
UI Preview
Please add screenshots for UI changes
Summary by CodeRabbit
New Features
Refactor