Stop scroll jump on workspace invite member selection#91250
Conversation
Keep selected items in the contacts section instead of separating them into a dedicated section. Use initiallyFocusedKey to scroll to the first selected item on mount, and prevent auto-scroll on subsequent selections. Co-authored-by: mkhutornyi <mkhutornyi@users.noreply.github.com>
|
All CI checks have passed except PR Author Checklist, which requires manual completion by the PR author — it's not a code issue. No fix is needed. |
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
|
@MelvinBot non-existing users disappear after selection. They should still be in top section (not in Contacts section). Screen.Recording.2026-05-21.at.4.41.25.AM.mov |
When a non-existing user (typed email) is selected via userToInvite, they are not part of personalDetails and would disappear from the UI. This adds them back in a top section above Contacts using selectedOptionsForDisplay filtered to exclude users already shown in the Contacts section. Co-authored-by: mkhutornyi <mkhutornyi@users.noreply.github.com>
|
Fixed in ab2c161. Non-existing users (typed emails selected via The issue was that |
| onSelectRow={handleToggleSelection} | ||
| shouldShowTextInput | ||
| textInputOptions={textInputOptions} | ||
| initiallyFocusedItemKey={initiallyFocusedKey} |
There was a problem hiding this comment.
@MelvinBot remove initiallyFocusedKey. We no longer need this.
There was a problem hiding this comment.
Already removed — it was cleaned up in the prior commit.
| }); | ||
|
|
||
| // Selected non-existing users that aren't in the Contacts section (e.g. typed email addresses) | ||
| const selectedNonExistingUsers = useMemo(() => { |
There was a problem hiding this comment.
@MelvinBot no need useMemo. Auto memoized by React Compiler
There was a problem hiding this comment.
Done — removed useMemo wrapper in 231b758.
No longer needed since selected items stay in-place in the Contacts section rather than being moved to a separate top section. Co-authored-by: mkhutornyi <mkhutornyi@users.noreply.github.com>
|
Removed |
React Compiler auto-memoizes this, so manual useMemo is unnecessary. Co-authored-by: mkhutornyi <mkhutornyi@users.noreply.github.com>
|
@MelvinBot I noticed that this page is re-rendered endlessly. |
Move selectedNonExistingUsers computation inside the sections useMemo and use selectedOptions (stable React state) instead of selectedOptionsForDisplay (new array reference every render) to avoid triggering unnecessary recomputations. Co-authored-by: mkhutornyi <mkhutornyi@users.noreply.github.com>
|
The excessive re-renders were caused by Fixed in a9ef427 by moving the computation inside the Note: |
|
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. |
|
@MelvinBot it's not caused by |
|
Understood — the excessive re-renders are a pre-existing issue in |
|
@MelvinBot non-existing user is duplicated briefly upon selection.
Screen.Recording.2026-05-21.at.4.58.29.AM.mov |
When shouldKeepSelectedInAvailableOptions is true, the hook does not null out userToInvite on selection. This caused duplicates — the user appeared in both the top selected section and the userToInvite section. Now we skip the userToInvite section when isSelected is true. Co-authored-by: mkhutornyi <mkhutornyi@users.noreply.github.com>
|
Fixed in f3c4143. The |
|
@MelvinBot when I type another user after non-existing user is selected, non-existing user is always visible despite that user doesn't match search keyword. |
Use selectedOptionsForDisplay instead of selectedOptions when computing the top section for non-existing users. selectedOptionsForDisplay is already filtered by the current search term, so non-existing users will correctly hide when they don't match the search keyword. Co-authored-by: mkhutornyi <mkhutornyi@users.noreply.github.com>
|
Fixed in c893bb3. The top section was using |
| const personalDetailLogins = new Set(availableOptions.personalDetails.map((option) => option.login).filter(Boolean)); | ||
| const selectedNonExistingUsers = selectedOptionsForDisplay.filter((option) => !personalDetailLogins.has(option.login)); |
There was a problem hiding this comment.
@MelvinBot move this logic to useSearchSelectorBase to be reusable. Introduce new param with default value = false
There was a problem hiding this comment.
Done in a005d57. Moved the logic into useSearchSelectorBase as a new shouldSeparateNonExistingSelectedOptions param (default false). When enabled, the hook computes and returns selectedNonExistingOptions — selected options not present in availableOptions.personalDetails.
Add shouldSeparateNonExistingSelectedOptions param (default false) to useSearchSelectorBase that computes and returns selectedNonExistingOptions — selected options not present in availableOptions.personalDetails. This makes the logic reusable across consumers. Co-authored-by: mkhutornyi <mkhutornyi@users.noreply.github.com>
| onSelectRow={handleToggleSelection} | ||
| shouldShowTextInput | ||
| textInputOptions={textInputOptions} | ||
| shouldPreventAutoScrollOnSelect |
Matches the pattern used in SearchMultipleSelectionPicker, SearchFiltersCardPage, and SearchFiltersWorkspacePage which all pass shouldUpdateFocusedIndex alongside shouldPreventAutoScrollOnSelect. Co-authored-by: mkhutornyi <mkhutornyi@users.noreply.github.com>
|
@mkhutornyi 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] |
|
@MelvinBot fix failing tests |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppandroid1.movandroid2.movAndroid: mWeb ChromeiOS: HybridAppios.moviOS: mWeb SafariMacOS: Chrome / Safariweb.mov |
- Rename ANOTHER_CONTACT to SECOND_CONTACT to avoid triggering
no-negated-variables rule ("another" contains "not")
- Use .at(0) instead of [0] for array access per prefer-at rule
Co-authored-by: mkhutornyi <mkhutornyi@users.noreply.github.com>
|
@francoisl 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] |
|
Fixed the ESLint errors in 976f11a:
|
| title: translate('common.contacts'), | ||
| data: availableOptions.personalDetails, | ||
| sectionIndex: 1, | ||
| sectionIndex: selectedNonExistingOptions.length > 0 ? 1 : 0, |
There was a problem hiding this comment.
@MelvinBot is it necessary to set sectionIndex conditionally based on selectedNonExistingOptions count?
sectionIndex only needs to be unique per section for key generation, not sequential. Use fixed values since only sections with data are pushed to the array. Co-authored-by: mkhutornyi <mkhutornyi@users.noreply.github.com>
|
No — |
|
@MelvinBot update |
|
Updated the Explanation of Change to reflect all current changes, including the new |
|
@francoisl while testing invite members page, I found bad bug on bug.mov |
|
Hm interesting, I'm not seeing that rerender issue. Neither on main nor on this branch. If you can consistently reproduce and find the root cause, then sure - feel free to make a follow-up PR. |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚧 @francoisl has triggered a test Expensify/App build. You can view the workflow run here. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
🚀 Deployed to staging by https://github.com/francoisl in version: 9.3.79-1 🚀
Bundle Size Analysis (Sentry): |
|
No help site changes are required for this PR. The changes are a UX improvement to the Workspace Invite Member selection list (preventing scroll jumps when selecting contacts). The relevant help article (Managing Workspace Members) describes the invite flow at a high level ("Search for the user by name, email, or phone number" → "Invite") and does not reference the specific selection list behavior that was changed. No workflow steps, feature names, button labels, or UI paths were affected. |
|
🚀 Deployed to production by https://github.com/roryabraham in version: 9.3.79-4 🚀
|
4 participants
Explanation of Change
Stop scrolling and jumping when selecting items on the Workspace Invite page (
DynamicWorkspaceInvitePage).Previously, selected contacts were separated into a dedicated section at the top, and selecting a new contact triggered an auto-scroll to the top of the list. This was disorienting when selecting multiple contacts from a long list.
Changes:
shouldKeepSelectedInAvailableOptions: truetouseSearchSelectorso selected items remain in the Contacts section with theirisSelectedflag instead of being moved to a separate sectionshouldSeparateNonExistingSelectedOptionsoption touseSearchSelectorBasethat computesselectedNonExistingOptions— selected users not present inavailableOptions.personalDetails(e.g. typed email addresses). These are shown in a dedicated top section above Contacts so they remain visible after selectionuserToInviterow when it is already selected to prevent duplicate entriesshouldUpdateFocusedIndexandshouldPreventAutoScrollOnSelectto the selection list to maintain scroll position when selecting itemsThis follows the same pattern applied in previous PRs for
SearchMultipleSelectionPicker,SearchFiltersCardPage, andSearchFiltersWorkspacePage.Fixed Issues
$ #61414
Tests
Offline tests
QA Steps
newuser@test.com) and select it — verify it appears in the top section (above Contacts), not inside the Contacts sectionPR 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
Screenshots will be provided in the reviewer checklist.
Android: Native
Will be added in reviewer checklist.
Android: mWeb Chrome
Will be added in reviewer checklist.
iOS: Native
Will be added in reviewer checklist.
iOS: mWeb Safari
Will be added in reviewer checklist.
MacOS: Chrome / Safari
Will be added in reviewer checklist.
AI Tests
prettierlint-changedtypecheck-tsgoreact-compiler-compliance-checktypecheck (tsc)