Workaround for programmatically setting focus on multi-select#4162
Open
Workaround for programmatically setting focus on multi-select#4162
Conversation
Contributor
📝 WalkthroughWalkthrough
ChangesFocus Event Handling in MultipleSelect
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Review rate limit: 0/1 reviews remaining, refill in 60 minutes.Comment |
Magnusrm
added
kind/bug
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/layout/MultipleSelect/MultipleSelectComponent.tsx`:
- Line 39: The delayed synthetic focusin handler can replay from an earlier
focus cycle and cause duplicate wake-ups; to fix, attach a per-focus-cycle token
(e.g., a numeric counter or timestamp stored in a ref alongside isPatchingFocus)
that you increment on each focus start (or when scheduling the delayed callback)
and capture into the scheduled callback closure; before dispatching the
synthetic focusin (the code that currently uses document.activeElement check),
compare the captured token against the current ref and bail out if they differ,
and ensure you update/reset the token on blur so stale timers cannot fire for a
new focus cycle.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: dee9c046-9ad7-4e75-aaf2-3177d05cf7e2
📒 Files selected for processing (1)
src/layout/MultipleSelect/MultipleSelectComponent.tsx
| } = useGetOptions(baseComponentId, 'multi'); | ||
| const groupBinding = useSaveValueToGroup(dataModelBindings); | ||
| const selectedValues = groupBinding.enabled ? groupBinding.selectedValues : selectedFromSimpleBinding; | ||
| const isPatchingFocus = useRef(false); |
Contributor
There was a problem hiding this comment.
Invalidate stale focusin replays across focus cycles.
Line 165 schedules a delayed synthetic focusin, but that callback is not tied to the focus event that created it. If the field blurs and regains focus within 150 ms, the older callback can still pass the document.activeElement check and fire during the newer focus cycle, producing duplicate wake-ups.
♻️ Suggested guard against stale callbacks
- const isPatchingFocus = useRef(false);
+ const isPatchingFocus = useRef(false);
+ const focusPatchSequence = useRef(0);
...
onFocus={async (e) => {
// Workaround for when programmatically focused by repeating group focus management
// If this event was triggered by our code below, reset the flag and exit.
if (isPatchingFocus.current) {
isPatchingFocus.current = false;
return;
}
- const input = e.target;
+ const input = e.currentTarget;
+ const sequence = ++focusPatchSequence.current;
// Wait for the combobox to be fully defined
await customElements.whenDefined('u-combobox');
setTimeout(() => {
+ if (focusPatchSequence.current !== sequence) {
+ return;
+ }
+
// Ensure we are still the active element
if (document.activeElement !== input) {
return;
}
// Tell the next execution of onFocus to ignore the event we are about to fire
isPatchingFocus.current = true;Also applies to: 151-176
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/layout/MultipleSelect/MultipleSelectComponent.tsx` at line 39, The
delayed synthetic focusin handler can replay from an earlier focus cycle and
cause duplicate wake-ups; to fix, attach a per-focus-cycle token (e.g., a
numeric counter or timestamp stored in a ref alongside isPatchingFocus) that you
increment on each focus start (or when scheduling the delayed callback) and
capture into the scheduled callback closure; before dispatching the synthetic
focusin (the code that currently uses document.activeElement check), compare the
captured token against the current ref and bail out if they differ, and ensure
you update/reset the token on blur so stale timers cannot fire for a new focus
cycle.
|
JamalAlabdullah
requested changes
May 4, 2026
Contributor
JamalAlabdullah
left a comment
JamalAlabdullah
left a comment
There was a problem hiding this comment.
Looks good and works 👍 , but maybe needs to check coderabbitai feedback and add test for it
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
2 participants
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.
Description
Related Issue(s)
Verification/QA
kind/*andbackport*label to this PR for proper release notes groupingSummary by CodeRabbit