Skip to content

fix(tags): prevent focus-back loop in eventCoalescing mode (#DS-5236)#1687

Open
NikGurev wants to merge 2 commits into
mainfrom
bugfix/DS-5236
Open

fix(tags): prevent focus-back loop in eventCoalescing mode (#DS-5236)#1687
NikGurev wants to merge 2 commits into
mainfrom
bugfix/DS-5236

Conversation

@NikGurev

@NikGurev NikGurev commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Summary

Applied fix to prevent tag list for capturing focus. Works regardless of mode.

Initially, bug was reproduced only with provideZoneChangeDetection({ eventCoalescing: true }) since batch processing of events.

@NikGurev NikGurev requested review from artembelik and lskramarov July 2, 2026 11:44
@NikGurev NikGurev self-assigned this Jul 2, 2026
@NikGurev NikGurev added the bug Something isn't working label Jul 2, 2026
@github-actions

github-actions Bot commented Jul 2, 2026

Copy link
Copy Markdown

Visit the preview URL for this PR (updated for commit 2a640f4):

https://koobiq-next--prs-1687-64hoeuk0.web.app

(expires Tue, 07 Jul 2026 14:57:42 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: c9e37e518febda70d0317d07e8ceb35ac43c534c

// it back to the first tag when the user tabs out.
this.keyManager.tabOut.pipe(takeUntilDestroyed(this.destroyRef)).subscribe(() => {
this._tabIndex = -1;
this.elementRef.nativeElement.tabIndex = -1;

@artembelik artembelik Jul 2, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'[attr.tabindex]': 'tabIndex',

tabIndex - же применяется к хосту, зачем перезаписывать это значение? возможно дело в cdr? миграция на сигнал не решит проблему?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

не решит,в режиме eventCoalescing keydown событие и focus tag-list будут объединены в группу, а CD произойдет после. Так же и при использовании сигнала

можно решить, добавив changeDetectorRef.detectChanges(), но избыточно если eventCoalescing: false. Поэтому установка tabIndex -1 - самый простой и рабочий способ

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a focus-back loop in KbqTagList when users tab out of the list (notably reproducible with provideZoneChangeDetection({ eventCoalescing: true })) by synchronously updating the host element’s native tabIndex so the browser won’t treat the list host as a tab stop during Tab key processing.

Changes:

  • Synchronously sets the host element’s native tabIndex = -1 on FocusKeyManager.tabOut to prevent the browser from re-focusing the tag-list host.
  • Adds a unit test asserting the native DOM tabIndex is updated immediately on tabOut.
  • Adds a unit test asserting tabIndex is restored to a user-provided value after tabOut.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
packages/components/tags/tag-list.component.ts Writes nativeElement.tabIndex = -1 synchronously on tabOut to avoid focus re-capture under event coalescing.
packages/components/tags/tag-list.component.spec.ts Adds coverage for immediate native tabIndex update and restoring user tabIndex after tab-out.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread packages/components/tags/tag-list.component.spec.ts
@NikGurev NikGurev requested a review from artembelik July 2, 2026 15:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants