fix: Add inline editing (Followup)#88722
fix: Add inline editing (Followup)#88722mohammadjafarinejad wants to merge 5 commits intoExpensify:mainfrom
Conversation
|
@ahmedGaber93 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] |
Co-authored-by: Copilot <copilot@github.com>
Co-authored-by: Copilot <copilot@github.com>
Codecov Report❌ Looks like you've decreased code coverage for some files. Please write tests to increase, or at least maintain, the existing level of code coverage. See our documentation here for how to interpret this table.
|
Co-authored-by: Copilot <copilot@github.com>
|
Hey! I see that you made changes to our Form component. Make sure to update the docs in FORMS.md accordingly. Cheers! |
Co-authored-by: Copilot <copilot@github.com>
|
Investigate some issue with Melvin until new PR is opened |
|
@MelvinBot Can you add a proposal here (in this PR discussion) for this issue #88737? |
Proposal for #88737Scan - Merchant field shows "Scanning..." text when editing inline What is the root cause of that problem?
The RHP doesn't have this problem because it reads the raw transaction merchant ( What changes do you think we should make in order to solve the problem?In Alternatively, disable inline merchant editing entirely while What alternative solutions did you explore?
|
@MelvinBot implement that on this PR Note: we should do the same for input amount since it also display |
|
To disable inline editing for merchant and amount while scanning, the changes needed are in
This prevents the "Scanning..." placeholder from appearing as editable text in both fields. |
|
@MelvinBot add proposal here for #88657 |
Proposal for Expensify/App#88657Expense - Significant delay to show Popover of category or date What is the root cause of that problem?
When the user dismisses a popover (clicks outside), What changes do you think we should make in order to solve the problem?Replace const startEditing = () => {
setIsEditing(true);
// Defer opening until next frame so the anchor is measured correctly
requestAnimationFrame(() => {
openPopover();
});
};This eliminates the dependency on the ReanimatedModal close animation's interaction handle lifecycle entirely. What alternative solutions did you explore?
Next Steps for Contributor+ team: Reply with |
|
@MelvinBot add proposal here for #88652 |
Proposal for #88652Expense - Merchant cannot be cleared by inline editing What is the root cause of that problem?
After the silent rejection, The existing non-inline editing path ( What changes do you think we should make in order to solve the problem?In
This aligns with the RHP editing behavior and the comment from What alternative solutions did you explore?
Next Steps for Contributor+ team: Reply with |
|
@mohammadjafarinejad I investigate another fix instead of edit
We will use the entire policy in the Then pass it here App/src/hooks/useTransactionInlineEdit.ts Line 169 in 8a0d1bf App/src/hooks/useTransactionInlineEdit.ts Line 151 in 8a0d1bf #88722 (comment) LGTM for #88737 #88722 (comment) LGTM for #88657 |
|
Fix for #88652 edit |
|
Fix for #88748 Disable category editing when Changespass canEditCategory will be: - canEditCategory:
- (isPolicyExpenseChat && (!!categoryForDisplay || hasEnabledOptions(policyCategories ?? {}))) || (isUnreported && (!policy || hasEnabledOptions(policyCategories ?? {}))),
+ canEditCategory:
+ (isPolicyExpenseChat && (!!categoryForDisplay || hasEnabledOptions(policyCategories ?? {}))) ||
+ // For unreported expenses, disable inline category editing while workspace selection is required.
+ (isUnreported && !shouldSelectPolicyForUnreported && hasEnabledOptions(policyCategories ?? {})),
|
|
@MelvinBot add proposal here for #88640 (comment) |
Proposal for #88640Report - Amount field accepts 0 amount in IOU expense when inline editing What is the root cause of that problem?The inline amount editing path lacks the zero-amount validation that the RHP already has. Specifically:
Meanwhile, the RHP correctly rejects zero amounts for P2P requests via What changes do you think we should make in order to solve the problem?Add a zero-amount guard in This is the correct layer for the fix because all inline amount edits flow through this action, making it a single point of enforcement. What alternative solutions did you explore?
Next Steps for Contributor+ team: Reply with |
|
Fix for #88640 function editTransactionAmountInline(params: TransactionInlineEditParams, newAmount: number) {
- if (newAmount < 0) {
+ if (newAmount < 0 || (newAmount === 0 && isIOUReport(params.parentReport))) {
return;
} |
Explanation of Change
This PR is a follow-up to fix deploy blockers related to the inline editing feature.
Fixed Issues
$ #88711
#82534
PROPOSAL:
Tests
Offline tests
QA Steps
// TODO: These must be filled out, or the issue title must include "[No QA]."
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari