Skip to content

Add automatic combobox item ID #97

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
henrycatalinismith wants to merge 1 commit into
base: main
Choose a base branch
from
+3 −0
Open

Add automatic combobox item ID #97

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,9 @@ export default class Combobox {
el.removeAttribute('data-combobox-option-default')

if (target === el) {
if (!target.id) {
target.id = `combobox-item-${Math.random().toString().slice(2, 6)}`
}
Comment on lines +129 to +131
Copy link

Copilot AI Mar 29, 2026

Choose a reason for hiding this comment

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

The auto-generated option id uses only a short Math.random().toString().slice(2, 6) fragment and doesn’t check for existing IDs in the document. This makes duplicate IDs very likely as users navigate through many options (and Number#toString() can sometimes yield fewer than 4 digits, shrinking the space further), which can break aria-activedescendant resolution and violates unique-id requirements.

Suggested fix: generate IDs in a much larger space and guarantee uniqueness (e.g., prefix with this.list.id and use a per-instance counter, or use crypto.randomUUID() when available with a fallback), and/or loop until document.getElementById(candidate) is null before assigning.

Copilot uses AI. Check for mistakes.
Comment on lines +129 to +131
Copy link

Copilot AI Mar 29, 2026

Choose a reason for hiding this comment

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

This change adds new behavior (auto-assigning an id when an option lacks one) but the test suite currently only covers options that already have explicit IDs. Please add a test case with a [role="option"] element missing id to assert that navigating sets a generated id and that aria-activedescendant points to it.

Copilot uses AI. Check for mistakes.
this.input.setAttribute('aria-activedescendant', target.id)
target.setAttribute('aria-selected', 'true')
fireSelectEvent(target)
Expand Down
Loading