Repository navigation
Revert "Restore: Show up to three report fields per row and edit them inline" - #103929
Conversation
|
Hey! I see that you made changes to our Form component. Make sure to update the docs in FORMS.md accordingly. Cheers! |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
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.
|
There was a problem hiding this comment.
💡 Codex Review
App/src/pages/DynamicEditReportFieldPage.tsx
Lines 169 to 171 in 253253c
When an administrator deletes a required date field from the workspace, its copy on an existing report can remain with deletable: false. Gating the menu on that stale flag removes the only deletion path, while handleReportFieldChange also ignores an empty value, so the obsolete field remains on the report permanently. Determine removability from whether the field still exists in the policy rather than relying only on the report copy's deletable flag.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| <OfflineWithFeedback | ||
| // Need to return undefined when we have pendingAction to avoid the duplicate pending action | ||
| pendingAction={pendingAction ? undefined : report?.pendingFields?.[reportField.fieldKey as keyof typeof report.pendingFields]} | ||
| errorRowStyles={styles.ph5} | ||
| key={`menuItem-${reportField.fieldKey}`} | ||
| onClose={() => clearReportFieldKeyErrors(report?.reportID, reportField.fieldKey)} |
There was a problem hiding this comment.
Pass report-field save errors to the feedback wrapper
When an UpdateReportField request fails after returning from the editor, the failure data writes report.errorFields[fieldKey] and rolls back the optimistic value, but this OfflineWithFeedback no longer receives that error. On multi-expense report views the save therefore appears to silently revert, unlike MoneyReportView, which still passes the same error field. Pass errors={report?.errorFields?.[reportField.fieldKey]} here so users can see and dismiss the failure.
Useful? React with 👍 / 👎.
| addBottomSafeAreaPadding: isSmallScreenWidth && !isKeyboardActive, | ||
| addOfflineIndicatorBottomSafeAreaPadding: false, | ||
| }); | ||
| const bottomSafeAreaPaddingStyle = useBottomSafeSafeAreaPaddingStyle({addBottomSafeAreaPadding: isSmallScreenWidth && !isKeyboardActive}); |
There was a problem hiding this comment.
Disable offline-indicator padding inside filter popovers
On a narrow screen while offline, this default enables addOfflineIndicatorBottomSafeAreaPadding, causing the hook to add CONST.OFFLINE_INDICATOR_HEIGHT to the popover content. The filter popover does not render an offline indicator, so users get a spurious blank gap that reduces the usable option-list area. Continue passing addOfflineIndicatorBottomSafeAreaPadding: false for this popover.
Useful? React with 👍 / 👎.
|
No product review needed |
|
🚧 AndrewGable has triggered a test Expensify/App build. You can view the workflow run here. |
|
@AndrewGable looks like this was merged without a test passing. These checks had not passed on 253253c when it was merged:
Please add a note explaining why this was done and remove the |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
Straight revert, no emergency. |
…FieldsGridInlineEditing-restore Revert "Restore: Show up to three report fields per row and edit them inline" (cherry picked from commit c456cd8) (cherry-picked to staging by AndrewGable)
|
🚀 Cherry-picked to staging by https://github.com/AndrewGable in version: 9.5.8-4 🚀
|
|
No help site update needed. This PR reverts Restore: Show up to three report fields per row and edit them inline, which never reached production, and no article in |
|
🚀 Deployed to production by https://github.com/cristipaval in version: 9.5.8-5 🚀
Bundle Size Analysis (Sentry): |
|
🚀 Cherry-picked to staging by https://github.com/AndrewGable in version: 9.5.9-0 🚀
|
|
No help site update needed. This PR and Restore: Show up to three report fields per row and edit them inline, the change it reverts, both reached production in the same release (9.5.8-5), so users never saw the change, and no article in |
|
🚀 Deployed to production by https://github.com/cristipaval in version: 9.5.9-4 🚀
Bundle Size Analysis (Sentry): |
Reverts the revert in Expensify#103929, putting back the changes from Expensify#102142 unchanged so the deploy blockers can be fixed one by one. Co-authored-by: daledah <daledah@users.noreply.github.com>
Straight reverts #102142
$ #103913
$ #103917
$ #103918
$ #103926