Skip to content
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.

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

[Bug]: Combobox throws errors when options with numeric values are provided #4384

Closed
1 task done
Rocss opened this issue May 2, 2024 · 3 comments · Fixed by #4405
Closed
1 task done

[Bug]: Combobox throws errors when options with numeric values are provided #4384

Rocss opened this issue May 2, 2024 · 3 comments · Fixed by #4405
Labels
bug Something isn't working triage An issue needing triage

Comments

@Rocss
Copy link
Contributor

Rocss commented May 2, 2024

Code of conduct

  • I agree to follow this project's code of conduct.

Impacted component(s)

sp-combobox

Expected behavior

Given that I have a list of options with numeric values, when I navigate via keyboard through the options, the list should scroll and no errors should be thrown.

Actual behavior

An error is being thrown in the console and the list does not scroll.

Screenshots

Screen.Recording.2024-05-02.at.13.37.43.mov

What browsers are you seeing the problem in?

Chrome

How can we reproduce this issue?

  1. Go here: https://opensource.adobe.com/spectrum-web-components/storybook/index.html?path=/story/combobox--controlled
  2. Navigate the options list using the keyboard.
  3. Check console.
  4. Observe the error thrown in the console.

Sample code that illustrates the problem

The Combobox code does

        const activeEl = this.shadowRoot.querySelector(
            `#${this.activeDescendant?.value}`
        ) as HTMLElement;

but for instances where value starts with a digit querySelector on IDs that start with a digit is not a valid selector.

Maybe querying by the value attribute or querying by [id="${this.activeDescendant?.value}"] would work better here?

Logs taken while reproducing problem

Uncaught DOMException: Failed to execute 'querySelector' on 'DocumentFragment': '#55' is not a valid selector. is being thrown

@Rocss Rocss added bug Something isn't working triage An issue needing triage labels May 2, 2024
@Westbrook
Copy link
Contributor

In theory, if we change:

 const activeEl = this.shadowRoot.querySelector(
            `#${this.activeDescendant?.value}`
        ) as HTMLElement;

to

 const activeEl = this.shadowRoot.getElementById(
            this.activeDescendant?.value
        ) as HTMLElement;

This would no longer throw and no practical changes would be needed in the template?

@Westbrook
Copy link
Contributor

There's the edge case where it tries to get an activeDescendant that doesn't exist, but I'm not sure how practical actually hitting the code path is...

@Rocss
Copy link
Contributor Author

Rocss commented May 2, 2024

getElementById works too (even with whitespaces, which is another edge).

Another thing I noticed here is that when I select an option using the keyboard, the change event is not triggered.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working triage An issue needing triage
Projects
None yet
2 participants