Repository navigation
Show up to three report fields per row and edit them inline - #100735
Conversation
Co-authored-by: daledah <daledah@users.noreply.github.com>
|
I created this PR from an upstream branch since I don't have push access to your fork. To take ownership of this branch and be able to push updates, run: Then you can close this PR and open a new one from your fork. |
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.
|
|
🤖 Web test steps verification results: ❌ fail I could not verify any of this PR's behavior on Web. The browser session left the app during fixture setup and landed on the ExpensifyHelp site, which ends the run — the session cannot be re-opened without dropping auth into a magic-code wall this environment can't clear. No screenshots or recordings of the report fields grid or inline editing exist, because the test never reached that screen. Two things also blocked me before the session was lost, and they need your input: 1. This PR has no documented testing steps. The 2. The fixture is expensive. Report fields only exist on a Control workspace, so verifying this needs a new workspace upgraded to Control, Report fields enabled, and at least four fields created across the Note that the PR body already states browser verification was not completed on the authoring run either, so the grid layout, inline saving, and narrow-layout single column remain unverified in a browser by anyone. To get this verified, fill in the view run · no recording available |
…t, apply oxfmt Co-authored-by: daledah <daledah@users.noreply.github.com>
|
Fixed the three failing checks in e0efa1c. All were caused by this PR's own changes.
Details and local verificationtypecheck — Oxfmt — oxfmt moved the knip — nothing imports Verified locally after the fix:
Not fixed, and not a failure: raw
The browser verification gap called out in the PR description still stands — the grid layout and inline saving have not been confirmed in a browser and need a human pass. |
|
🤖 Android test steps verification results: ✅ pass On standalone Android NewDot, all four report field types (text, list/dropdown, date, formula) render one-per-row as inline editable inputs on a Control-workspace expense report. Inline text/list/date editing, no-op refocus/blur, formula read-only, and required-field validation all behaved correctly, with no crashes or red boxes. Two caveats worth reading before you treat this as full coverage — they are below the table. Two caveats:
Setup for reproducibility: created a Control-plan workspace, enabled Report fields, and added a text field ("Project Name", required), a list field ("Region" with Asia/Europe/North America), a date field ("Trip Date") and a formula field ("Computed Total") before creating the test expense report. |
|
@MelvinBot The 3-per-row grid doesn't render. Every field still takes a full row on a wide screen. Root cause: Please use |
…eal pane width Co-authored-by: daledah <daledah@users.noreply.github.com>
|
@MelvinBot The date fields render with no calendar icon, so they don't match the mock.
Fix it in - icon={selectedDate ? null : icons.Calendar}
+ icon={selectedDate && !shouldHideClearButton ? null : icons.Calendar} |
|
No code fix is needed — every code check on this PR passes. 34 of 35 checks pass. The one failure is All code-quality and test checks are green: Two things still blocking this PR that a human needs to do:
Also worth flagging from the PR body: browser verification of the grid layout and inline editing was never completed, so the three-per-row layout, inline saving, and the narrow-layout single-column fallback are still unverified in a real browser. view run · no recording available |
|
Confirmed and fixed in c193348. Both report screens are in Change and test details
Browser verification is still not done. The pinned web session's Chrome process was relaunched on a fresh profile mid-run, which dropped auth, and the replacement page never rendered the app (0 accessibility nodes, blank frame, all waits timed out). The dev server was healthy and rebuilt the changed file with no errors, so there's no evidence of a runtime problem — but the three-per-row grid, inline saving and the narrow-layout fallback are still only covered by unit tests and need a human browser pass. |
Co-authored-by: daledah <daledah@users.noreply.github.com>
|
Re-checked on the current head ( All 34 code checks pass on The single failure is still Same two human steps as before: fill in the |
|
Applied your fix in 49fd55e, and this time the browser verification went through — the calendar icon now stays on the date field after it holds a value.
Checks: One thing outside your ask, flagged in case it matters: empty text report fields render the red "required" error immediately on report creation, before the user touches them. |
|
@MelvinBot Two problems with the list-type fields. 1. No caretThe LIST branch renders its + icon={icons.DownArrow}
+ iconContainerStyle={[styles.pr0, isExpanded && styles.flipUpsideDown]}2. It opens a page, not a dropdownThe branch mounts
Keep VerificationBoth changes were implemented and checked locally |
…of an RHP page Co-authored-by: daledah <daledah@users.noreply.github.com>
|
MelvinBot has made 5 consecutive commits trying to fix CI failures. Automatic CI fix dispatches are now paused for this PR. Next steps: A human should review the recent commits and resolve the conflicting CI errors manually. |
|
Both done in 424abaa. The
One deviation: the popover wrapper uses Other details and checks
Browser verification failed again, and this time my own edits caused it. The pinned session was handed over healthy, but saving these files fired six HMR rebuilds of |
@MelvinBot, we don't have top padding on the search input, we can remove the recent section and hide the search input if the list has fewer than 8 items. |
…nd hide search for short lists Co-authored-by: daledah <daledah@users.noreply.github.com>
|
Done in 9a62cfa — all three.
Two things worth your call:
Also fixed, and test detailsThe popover height helper was reserving 64px for a search bar unconditionally via New
Browser verification is still blocked, for the second run in a row. The pinned web session's page drops to an empty DOM (blank frame, 0 accessibility nodes, every wait timing out) before the tester can issue its first command, and recovering needs a reload that would drop auth. The dev server serves the app shell fine and rsbuild rebuilt the changed files with no errors, so there's no evidence pointing at this code — but the padding, the hidden search input and the missing Recent section are unverified visually and need your eyes. |
|
@MelvinBot please open this PR |
|
🚀 Deployed to staging by https://github.com/Gonals in version: 9.4.91-0 🚀
|
|
🤖 Yes — help site changes were required. Draft PR: Docs updates for E/A#100735 inline report field editing on reports ( This PR changed the member-facing flow: report fields on a report used to be full-width rows that opened the report field editor page, and they are now inputs on the report itself — up to three per row on a wide screen, one per row on a narrow screen or a single-expense report — with text, date and list values saved in place. The help site had nothing covering how a member fills out report fields on a report, so that behavior was undocumented. What the docs PR does
Verification and scope notes
@daledah, please review the linked help site PR and confirm it reflects the current behavior. Then mark the linked help site PR |
|
Any QA steps here? |
|
Deploy Blocker #101804 was identified to be related to this PR. |
|
Deploy Blocker #101805 was identified to be related to this PR. |
|
Deploy Blocker #101798 was identified to be related to this PR. |
|
Deploy Blocker #101799 was identified to be related to this PR. |
|
Deploy Blocker #101802 was identified to be related to this PR. |
|
Deploy Blocker #101806 was identified to be related to this PR. |
|
Deploy Blocker #101810 was identified to be related to this PR |
|
Deploy Blocker #101823 was identified to be related to this PR |
|
Deploy Blocker #101824 was identified to be related to this PR |
|
🚀 Deployed to production by https://github.com/lakchote in version: 9.4.91-3 🚀
Bundle Size Analysis (Sentry): |













Explanation of Change
Report fields used to render as a vertical stack of full-width rows, each one navigating to the report field editor page to change a value. With nine report fields that filled the whole screen and pushed the expense list out of view, and every edit cost a navigation.
This changes both halves of that, per the approved proposal:
Up to three fields per row.
MoneyRequestViewReportFieldsnow chunks the sorted fields into rows ofCONST.REPORT_FIELDS_PER_ROW(3) and lays each row out as aflexRowof equal-width cells. A partly filled last row is padded with empty cells so its fields keep the same width as the rows above. On a narrow layout the column count drops to 1. Both existing call sites — the report list header and the empty report view — render the same component, so they both pick this up.Inline editing. The new
ReportFieldInlineInputrenders each field as one of our regular 52px inputs instead of a row that navigates away:textTextInput, saved on blur and on submit, skipped when unchangeddateDatePicker— the calendar opens over the reportdropdownEditReportFieldDropdown, so search and recently-used values are preservedformulaText values keep the validation the editor page applied: required fields reject an empty value with
common.error.fieldRequired, and a value that makes a formula refer to itself is rejected withworkspace.reportFields.circularReferenceError.OfflineWithFeedback, the per-field pending state and the field violation message all work as before.Shared save path. The save logic that lived inline in
DynamicEditReportFieldPagemoved intouseSaveReportField. Both the editor page and the inline inputs call it, so the violation, ASAP-submit and recently-used handling only exists in one place.updateReportField'spolicyparameter was widened toOnyxEntry<Policy>to match how it already uses the value (policy?.), which removed the unsafe cast every caller needed.The editor page is unchanged from the user's point of view — it still handles deep links and remains the narrow-layout path for the report title.
Fixed Issues
$ #100259
PROPOSAL: #100259 (comment)
Tests
// TODO: The human co-author must fill out the tests you ran before marking this PR as "ready for review".
// Please describe what tests you performed that validate your change worked.
Offline tests
QA Steps
// TODO: These must be filled out, or the issue title must include "[No QA]."
AI Tests
Automated checks run locally on this branch:
npm run typechecknpm run lint-changednpm run react-compiler-compliance-check check(changed files)npm run spell-changednpm test -- tests/ui/MoneyRequestViewReportFieldsTest.tsx(new, 5 tests)npm test -- tests/ui/MoneyReportViewTest.tsx tests/actions/ReportFieldTest.ts tests/ui/DynamicReportDetailsPageTest.tsxnpm test -- tests/ui/MoneyRequestReportViewTest.tsx tests/ui/MoneyRequestReportTransactionsNavigationTest.tsxnpm run lint(full repo)lint-changedcovers every file in this diffBrowser verification could not be completed. The pinned web session went to a blank, unresponsive page after the dev server hot-reloaded these components, and reloading the tab is not permitted for this tester (it drops auth). The dev server log shows all four changed files rebuilt successfully with no compile error, and no console error or exception was captured, so there is no evidence of a runtime failure — but the three-per-row layout, inline saving and the narrow-layout single column are unverified in a browser and need a human pass.
New unit tests cover: three fields per row on a wide layout, one per row on a narrow layout, each field rendering as an input holding its value, a typed value saving through
updateReportField, and an unchanged value not saving.PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectionAvatar, 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.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