Repository navigation
[CFI] Open the Category list in a container anchored to its row - #102665
puneetlath merged 14 commits into
Conversation
The create expense form renders Category as a bordered field with a down caret, but the caret was cosmetic: tapping the row pushed a full-page selector. This makes the caret real for Category. - Adds ExpenseFieldDropdown, which pairs an ExpenseFieldRow with a container anchored to it. On a wide layout the container matches the row's width and opens directly below the row, flipping above it when there isn't room below, and is capped so it is never clipped. On a narrow layout PopoverWithMeasuredContent docks it to the bottom, so the bottom sheet is the existing pattern with no new component. - Reuses CategoryPickerModal, which already wraps CategoryPicker for the Search table's inline edit, rather than building a second list. It now takes the pop-over's width and height so a field row can hand it its own, and spans the screen when it renders as a bottom sheet. - Extracts the category write out of DynamicIOURequestStepCategory into useUpdateTransactionCategory, so a category picked in the dropdown lands exactly where the same pick on the full page lands: the split draft, the saved expense, or the money request draft. - Keeps the full-page selector for every case the container can't answer on its own: no workspace picked yet, an upgrade in the way, no list loaded, or a saved expense being edited from this form. The list is mounted only after the row is first opened, so a field nobody touches costs the form nothing. Fields other than Category, and the expense detail and bulk edit surfaces, are unchanged and follow separately. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.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.
|
The five list fields on the expense form land one at a time. A form where Category opens its list in place while the four rows under it still push a full page reads as broken, so the whole set stays dark until it is complete. canUseAnchoredFieldDropdowns follows the canUseTrackFlows / canUsePrivateNotes pattern already in Permissions: a plain client gate, no beta name and no Auth change, since nothing about this needs the server. It returns a widened boolean rather than a literal so the gated branches are not statically dead, which is what keeps the container reachable to test. It is on everywhere but production, so design can judge the container from an adhoc build while nobody else sees a half-converted form. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Knip flagged it as a new unused exported type. Only the render props are consumed outside the module, by CategoryFieldDropdown and the test; the props type is a leftover from an earlier draft of the test that reached for it through React.ComponentProps. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ight Two things made the container taller than it had any need to be, which showed up as it running past the RHP the expense form sits in. The container asked for a minimum height even when the row left it less room than that. Math.max(availableHeight, MIN_CONTAINER_HEIGHT) turned a 40px gap into a 180px request, and the popover's overflow shift then dragged it back over the row. The space the row leaves is a ceiling, not a target, so the floor is gone. The list itself was then stretched to fill whatever height came out of that: CategoryPickerModal draws its content at a fixed height, so a three-category workspace got the full 416px with most of it empty. It now estimates the height its list needs from the option count, the same way getSelectionListPopoverHeight estimates the Spend filters' popovers, and treats the height it was handed as the cap. Opt-in via shouldFitContentHeight so the Search table's inline edit keeps the size it has today. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Nothing about this feature needs the environment, so the gate does not need to read it. A plain false matches canUseTrackFlows and canUsePrivateNotes exactly, and the : boolean return type still widens it at the call sites, so the gated branches stay reachable to compile and test. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two changes to how the container picks its side. It now flips above the row as soon as below cannot hold the whole container, rather than waiting until below is nearly unusable. Below stays the default while it fits, so the common case is unchanged, but a row low down in a panel now opens upwards into the room it has instead of downwards into a sliver. Opening above also stops short of the page header. The header holds the back button, and the container was free to run over it, covering the way out of the page it belongs to. The space above the row is now measured from the bottom of the header rather than from the top of the window, using the same useContentHeaderHeight the header itself is drawn with, plus the top safe area inset. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
PopoverWithMeasuredContent can move itself to the other side of its anchor when it decides it overflows, and it does so by shifting a whole popover height. CategoryPickerModal had that switched on, so the container picked a side twice: once here from the row's measurements, then again inside the popover. The second decision had no idea the first had been made and landed the list on the row. The caller now turns it off, since it has already chosen the side. Two smaller holes fed the same symptom. The container's border sits outside the height it is given, so the height is now taken from the available space less that border. And a side with no usable room left the container with a height of zero or less, which reads as "no static dimensions" and makes the popover measure its own content instead, ignoring the ceiling entirely and then clamping itself back over the row to fit the window. A row with too little room on either side now opens its full-page selector instead. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
@ikevin127 @thelullabyy One of you needs to 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] |
|
Looks like you have some ESLint failures and the OP needs some vids, @DylanDylann. @ikevin127 if you want to begin a review in the meantime, please do. 👍 |
Reviewer Checklist
Screenshots/VideosiOS: NativeScreen.Recording.iPhone.17.Pro.Max.09-30-2026.at.20.22.21.mp4MacOS: Chrome / SafariScreen.Recording.2026-09-30.at.20.17.52.mov |
ikevin127
left a comment
There was a problem hiding this comment.
Tests well generally, but I found 6 actionable issues in this PR: 2 🟠, 3 🟡 and 1 🟢. ESLint is the only failing CI check, because usePermissions is imported but never used in CategoryField.tsx.
🔄 I left out what the bots and the author already raised: the em-dash comments, the duplicated SEARCH_INPUT_HEIGHT / CONTENT_VERTICAL_PADDING constants, and the author's plan to flip the flag before merge.
|
🚀 Deployed to staging by https://github.com/puneetlath in version: 9.5.3-0 🚀
|
|
No help site update needed. The new anchored Category list only appears behind the new |
|
Deploy Blocker #103124 was identified to be related to this PR. |
|
Deploy Blocker #103125 was identified to be related to this PR. |
|
Deploy Blocker #103129 was identified to be related to this PR. |
|
Deploy Blocker #103132 was identified to be related to this PR. |
|
Deploy Blocker #103131 was identified to be related to this PR. |
|
Deploy Blocker #103133 was identified to be related to this PR. |
|
Deploy Blocker #103136 was identified to be related to this PR. |
|
Deploy Blocker #103137 was identified to be related to this PR. |
|
Deploy Blocker #103140 was identified to be related to this PR. |
|
Deploy Blocker #103171 was identified to be related to this PR. |
|
🚀 Deployed to production by https://github.com/puneetlath in version: 9.5.3-4 🚀
Bundle Size Analysis (Sentry): |
Explanation of Change
Fixed Issues
$ #101573
PROPOSAL:
Tests
anchoredFieldDropdownsbeta to trueCategoryrow.Categoryrow sits near the bottom of the window, then click it. Verify the pop-over opens above the row instead, and is not clipped by the window.Offline tests
Same above
QA Steps
Same above
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectionAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))CategoryPickerModalis shared with the Search table's inline category edit and needs re-verifying there.Designlabel and/or tagged@Expensify/designso the design team can review the changes.tests/unit/components/ExpenseFieldDropdown.test.tsxcovers lazy mounting, opening in place vs. navigating, the row-width/below-the-row geometry, the flip above the row, dismissing by pressing the row again, and the full-page fallback.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
iOS: Native
Screen.Recording.2026-10-01.at.15.57.58.mov
MacOS: Chrome / Safari
Screen.Recording.2026-10-01.at.15.42.30.mov
🤖 Generated with Claude Code