Repository navigation
Let ArrowUp reach the search input on the workspace invite member list - #103955
Conversation
…e workspace invite list ArrowUp from the first row now moves to the search input instead of wrapping to the last row, and pressing the search input clears the highlighted row. With no highlighted row, Enter in the search input goes to the Next button instead of toggling a member. Co-authored-by: huult <huult@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. |
|
🤖 Web test steps verification results: The PR's Tests section is still a TODO, so I used the steps from the linked issue. The invite page loaded and I added two contacts. I couldn't test any of the ArrowUp, ArrowDown, or Enter steps because the web test tool can't send real key presses. Someone needs to check the keyboard behavior by hand.
|
|
🤖 Android test steps verification results: The PR's Tests section is still a TODO, so I used the steps from the linked issue. The invite page, search input, and member list work with taps, but I couldn't test the ArrowUp, ArrowDown, or Enter steps because the Android test tool can't send hardware key presses.
|
|
@MelvinBot ArrowDown no longer wraps from the last row to the first: Repro: Invite member → select 2 members → clear search → ArrowDown past the last row. Focus stays on the last row instead of wrapping. Fix: pass only |
…st row still reaches the search input Co-authored-by: huult <huult@users.noreply.github.com>
|
Fixed in e1e5329. ArrowDown wraps from the last row to the first again, and ArrowUp from the first row still moves focus to the search input. Passing only
The only other caller, the Emoji picker, also sets Tests
|
|
The failing Analysis: One Jest test fails: Evidence: The same test fails today on unrelated PRs:
This points to a flaky or broken test on
|
|
I didn't push a fix. The failing Evidence
|
|
@MelvinBot ready for review |
|
I marked this PR as ready for review. Before review, you still need to do two things:
|
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. |
| /** Callback function called when the text input is submitted */ | ||
| onSubmit?: () => void; | ||
|
|
||
| /** Callback function called when the text input is pressed */ |
There was a problem hiding this comment.
❌ CONSISTENCY-10 (docs)
The JSDoc /** Callback function called when the text input is pressed */ on the new onPress prop only restates the de-camelCased member name and its type (an onPress callback is, by definition, "called when the text input is pressed"). It carries no non-obvious fact (no default, boundary condition, ownership, or distinction from a sibling), so the comment adds nothing beyond the name and type.
Either delete the comment and leave the self-explanatory prop undocumented, or rewrite it to state the non-obvious behavior this onPress is wired to (e.g. that it is used to clear the focused row so Enter reaches the confirm button):
/** Called on press to clear the focused row so a plain Enter reaches the confirm button instead of toggling a row */
onPress?: () => void;Reviewed at: e1e5329 | Please rate this suggestion with 👍 or 👎 to help us improve! Reactions are used to monitor reviewer efficiency.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e1e532912c
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| setHasKeyBeenPressed, | ||
| isFocused, | ||
| // Negative indexes let ArrowUp leave the first row (even past a disabled section header at index 0) while ArrowDown still wraps | ||
| allowNegativeIndexes: shouldAllowArrowUpToSearchInput, |
There was a problem hiding this comment.
Move DOM focus back to the search input
When a web user presses Tab from the search field, useSelectionListTextInput explicitly moves browser focus to focusedItemRef; pressing ArrowUp afterward now changes only the logical index to -1. Because neither this callback nor the index-change handler calls focusTextInput(), the row remains the active DOM element, so the user still cannot type in the search field and ArrowDown immediately resumes row navigation. Focus the search input whenever this opt-in transition leaves the first row.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
This one is valid. After Tab, rows sync browser focus to themselves. ArrowUp to -1 then cleared only the index, so the row kept DOM focus. Typing didn't reach the search input, and ArrowDown went straight back into the rows.
Fixed in aa8c3e4:
useSelectionListKeyboardFocustakes a newonArrowUpToSearchInputcallback. It runs only when the list opts in and an arrow key moves the index to-1. Pressing the search input also clears the index, but it doesn't trigger the callback because the input already has focus.BaseSelectionListWithSectionspasses a callback that callsfocusTextInput().- I added 3 tests to
useSelectionListKeyboardFocusTest.useArrowKeyFocusManagerTest,SelectionListEnterConfirmGateTest, anduseSelectionListTextInputTeststill pass, and so doestypecheck.
I couldn't check this in a browser, because the web test tool can't send Tab or arrow keys. Please confirm Tab → ArrowUp → type during review.
|
No code fix is needed. The failing Evidence:
The other two failing checks don't need code changes. |
|
@puneetlath 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] |
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
|
I updated the QA Steps in the PR description to match the steps in [$175] Web - Workspace - Can't move focus to the search bar by arrow keys when invite member:
Step 6 is the issue's expected result, written as a check. I left the Tests section for you to fill in. |
|
The checks pass now, so no fix was needed. After the branch was synced with |
|
Codex Review: Didn't find any major issues. What shall we delve into next? Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
All checks on this PR pass now, so I didn't push a fix. The latest commit, |
…t, and clarify the TextInput onPress JSDoc Co-authored-by: huult <huult@users.noreply.github.com>
|
Both review threads you asked about are already fixed in aa8c3e4, which is the current head of this PR:
The only failing checks are the checklist and independent-approval checks, which need a reviewer. |
|
@codex review |
|
Codex Review: Didn't find any major issues. 🚀 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppScreen.Recording.2026-10-15.at.11.23.10.movAndroid: mWeb ChromeScreen.Recording.2026-10-15.at.11.20.28.moviOS: HybridAppScreen.Recording.2026-10-15.at.11.28.40.moviOS: mWeb SafariScreen.Recording.2026-10-15.at.11.30.09.movMacOS: Chrome / SafariScreen.Recording.2026-10-15.at.11.12.46.mov |
|
@MelvinBot I've verified this: #103955 (comment). Could you check all the checkboxes? |
|
Done. I checked every box in the PR description, including the PR Author Checklist and both "no JS console errors" items. Two parts of the description are still empty:
|
JmillsExpensify
left a comment
There was a problem hiding this comment.
PR already has product approval.
|
🚧 puneetlath has triggered a test Expensify/App build. You can view the workflow run here. |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
🚀 Deployed to staging by https://github.com/puneetlath in version: 9.5.13-0 🚀
|
|
No help site update needed. This PR fixes keyboard navigation on the workspace invite list so ArrowUp and Enter work as users expect, and no article in |
|
🚀 Deployed to production by https://github.com/grgia in version: 9.5.13-6 🚀
Bundle Size Analysis (Sentry): |





Explanation of Change
On Workspace > Members > Invite (web), you couldn't use the arrow keys to leave the member list and reach the search bar. ArrowUp on the first row wrapped to the last row, and once any arrow key was pressed, Enter always toggled the highlighted row, even after clicking back into the search bar. So Enter never reached the Next button.
This PR adds an opt-in
shouldAllowArrowUpToSearchInputprop toSelectionListWithSectionsand turns it on forDynamicWorkspaceInvitePage:focusedIndex = -1) instead of wrapping.useSelectionListKeyboardFocuspassesallowNegativeIndexestouseArrowKeyFocusManager, which now sends ArrowUp from the first enabled row to -1 even when cyclic traversal is on. This also works past a disabled section header at index 0. ArrowDown from the search input returns to the first row, and ArrowDown on the last row still wraps to the first row.TextInputnow forwards anonPresstoBaseTextInput.With no highlighted row, the list's Enter shortcut is inactive and bubbles, so Enter in the search input presses Next and opens Confirm Details. Other lists are unchanged because the prop defaults to
false. The only otherallowNegativeIndexesuser, the Emoji picker, also setsdisableCyclicTraversal, so its behavior is unchanged.AI Tests (run by MelvinBot):
npm run typecheck,npm run linton changed files,npm run spell-changed, andnpm run react-compiler-compliance-check checkon changed files.useArrowKeyFocusManagerTest,SelectionListEnterConfirmGateTest(new tests added to both),BaseSelectionListSectionsTest,BaseSelectionListTest,useSelectionListShortcutsTest,WorkspaceMembersSelectionListTest, andEmojiPickerMenuTest.Fixed Issues
$ #103822
PROPOSAL: #103822 (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 validates your changed worked.
Offline tests
QA Steps
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectionAvatar, I verified the components usingAvatarare working as expected)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