refactor(FR-3848): give the assign modal's user select its own Suspense boundary - #9419
Open
nowgnuesLee wants to merge 3 commits into
Open
refactor(FR-3848): give the assign modal's user select its own Suspense boundary#9419nowgnuesLee wants to merge 3 commits into
nowgnuesLee wants to merge 3 commits into
Conversation
…se boundary `AssignRoleModal` ran `useLazyLoadQuery` in the modal body, so a fresh mount suspended up to the tab-level boundary; #9413 worked around that by deferring `open` and reading the store only on the first pass. Express the boundary directly instead: the user options query, the search state and the `BAISelect` move into `AssignRoleUserSelect`, rendered inside its own `Suspense`. The boundary sits inside the `Form.Item` child so the `userIds` field stays registered while the options load; the fallback is the same select in `loading` + `disabled` state. `maxTagCount` / `maxTagPlaceholder` are dropped while moving the select — `BAISelect` accepts and ignores both — which also retires the `Tooltip` and `BAIFlex` imports. The modal only mounts while open, so the fetch policy is a fixed `store-and-network`. Co-Authored-By: Claude Fable 5.1 <[email protected]>
The wrapper component existed only to hold a Suspense around the select. Put the boundary inline where the select is rendered instead, wrapping the Form.Item the way UpdateUsersModal does, with a same-label loading select as the fallback. Co-Authored-By: Claude Fable 5.1 <[email protected]>
Contributor
Coverage Report for react-coverage (./react)
File Coverage
|
||||||||||||||||||||||||||||||||||||||
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The fallback unregisters required validation, allowing an empty assignment during loading.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Refactors the role assignment modal so user loading suspends only the select.
Changes:
- Extracts the queried user select.
- Adds an inline loading fallback.
- Removes deferred modal opening and ignored select props.
File summaries
| File | Description |
|---|---|
react/src/components/AssignRoleModal.tsx |
Adds the Suspense-bounded user select. |
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 1
- Review effort level: Balanced
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
The Suspense fallback rendered an unnamed Form.Item, so while the query suspended `validateFields()` skipped `userIds` and Assign could submit an empty list. Register the same field and required rule on the fallback. Co-Authored-By: Claude Fable 5.1 <[email protected]>
nowgnuesLee
marked this pull request as ready for review
September 3, 2026 10:11
agatha197
requested changes
Sep 3, 2026
| >; | ||
|
|
||
| /** Owns the user options query; render it under a Suspense boundary. */ | ||
| const AssignRoleUserSelect: React.FC<AssignRoleUserSelectProps> = ({ |
Contributor
There was a problem hiding this comment.
how about separate this select?
| {option.data?.description && ( | ||
| <div | ||
| style={{ | ||
| fontSize: 12, |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Resolves #9418 (FR-3848)
Follow-up to #9413 (FR-3725).
Why
#9413 stopped the Add user click from suspending the whole Role assignment tab by deferring the modal's
openinsideAssignRoleModal(useDeferredValue(open, false)plus astore-onlyfirst pass). That keeps the tab rendered, but it is an indirect way of saying "this query needs its own Suspense boundary".Change
react/src/components/AssignRoleModal.tsxonly.AssignRoleModalQuery), the search state and theBAISelectmove intoAssignRoleUserSelect. The deferred-open handling is removed.Form.Itemthat renders it is wrapped in aSuspenseinline in the form, the same shape asUpdateUsersModal. The fallback is aForm.Itemwith the same label holding the select inloading+disabledstate, so the modal opens at once with no layout shift.maxTagCount/maxTagPlaceholderare dropped while moving the select:BAISelectaccepts and ignores both. TheirTooltip/BAIFleximports go with them.BAIUnmountAfterClose), so the fetch policy is a fixedstore-and-network.The GraphQL document is unchanged, so no
__generated__output changes.Verification
bash scripts/verify.sh→=== ALL PASS ===🤖 Generated with Claude Code