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

Fixing indexOf usage bug when selection-list handles changes in items. #28531

Merged
merged 1 commit into from
Feb 8, 2024

Conversation

dyoo
Copy link
Contributor

@dyoo dyoo commented Feb 3, 2024

The buggy logic was using a truthiness check for checking for presence in a list; this check needs to compare against -1 instead. This bug causes mismanagement of tabIndex, which caused an accessibility bug where, if the list of items changed, the selection list could not be tab selected until the selection list was focused.

@dyoo dyoo changed the title Fixing indexOf bug when selection-list handles changes in items. Fixing indexOf usage bug when selection-list handles changes in items. Feb 4, 2024
@devversion devversion removed their request for review February 5, 2024 14:18
@devversion devversion closed this Feb 6, 2024
@devversion devversion reopened this Feb 6, 2024
@dyoo dyoo force-pushed the select-list-b323254994 branch 2 times, most recently from 0581edb to f97fef6 Compare February 6, 2024 10:16
The buggy logic was using a truthiness check for checking for presence
in a list, but this check needed to compare against -1 instead.  This
corrects a mismanagement of tabIndex on the items in the selection
list: if the list of items changed, the selection list could not
be tab selected until the selection list was mouse-focused.
Copy link
Member

@crisbeto crisbeto left a comment

Choose a reason for hiding this comment

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

LGTM

@crisbeto crisbeto removed the request for review from mmalerba February 8, 2024 13:24
@crisbeto crisbeto added action: merge The PR is ready for merge by the caretaker target: patch This PR is targeted for the next patch release labels Feb 8, 2024
@crisbeto crisbeto merged commit f1e82da into angular:main Feb 8, 2024
24 of 26 checks passed
crisbeto pushed a commit that referenced this pull request Feb 8, 2024
…28531)

The buggy logic was using a truthiness check for checking for presence
in a list, but this check needed to compare against -1 instead.  This
corrects a mismanagement of tabIndex on the items in the selection
list: if the list of items changed, the selection list could not
be tab selected until the selection list was mouse-focused.

(cherry picked from commit f1e82da)
crisbeto pushed a commit that referenced this pull request Feb 8, 2024
…28531)

The buggy logic was using a truthiness check for checking for presence
in a list, but this check needed to compare against -1 instead.  This
corrects a mismanagement of tabIndex on the items in the selection
list: if the list of items changed, the selection list could not
be tab selected until the selection list was mouse-focused.

(cherry picked from commit f1e82da)
@dyoo dyoo deleted the select-list-b323254994 branch February 9, 2024 19:35
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Mar 11, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
action: merge The PR is ready for merge by the caretaker target: patch This PR is targeted for the next patch release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants