Skip to content

fix(material/paginator): fix issue with paginator focus #29006

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

Closed
wants to merge 6 commits into from

Conversation

DBowen33
Copy link
Contributor

@DBowen33 DBowen33 commented May 6, 2024

Fixes an issue where focus is lost and goes back to the body when on the next, previous, first, or last buttons are disabled. Changed to where focus will now jump to the next appropriate button instead of focusing to body.

Before:
Screen recording 2024-05-07 9.22.24 AM.webm

After:
Screen recording 2024-05-07 9.19.26 AM.webm

Fixes b/286098030

DBowen33 added 6 commits May 6, 2024 20:36
Fixes and issue where focus is lost when on the next, previous, first, or last buttons are disabled.
changed to where focus will now jump to next appropriate button instead of focusing to body

Fixes b/286098030
refactoring code to make cleaner

Fixes b/286098030
refactored code to fix tslint error. added comments to explain code better

fixes b/286098030
remove HostListener import

fixes b/286098030
fix ts lint issue

fixes b/286098030
approved api golden

fixes b/286098030
@DBowen33 DBowen33 marked this pull request as ready for review May 7, 2024 18:18
@DBowen33 DBowen33 requested a review from crisbeto as a code owner May 7, 2024 18:18
@andrewseguin
Copy link
Contributor

This doesn't seem to capture the case of activating the button via other keypresses or clicks. It's also fragile depending on a 100ms delay to check whether the button becomes disabled and moving focus. Using afterNextRender would be more stable

It'd be nice if there was more intelligence in the paginator to know that a button has focus, but is about to lose focus so that it can directly move it. Right now it seems this change will see the focus move from the button, then to the body temporarily, then to another button.

@andrewseguin
Copy link
Contributor

How about adding a (blur) listener on the buttons that can check whether they are now disabled? If so, move focus to another element [that isn't disabled]

@andrewseguin
Copy link
Contributor

Another alternative is just to set disabledInteractive on the buttons, which lets them remain focusable after they become disabled

@josephperrott josephperrott requested a review from a team as a code owner December 18, 2024 17:40
@josephperrott josephperrott requested review from amysorto and removed request for a team December 18, 2024 17:40
@crisbeto crisbeto self-assigned this Dec 19, 2024
@crisbeto
Copy link
Member

Looks like this is resolved by #29379.

@crisbeto crisbeto closed this Dec 19, 2024
@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 Jan 19, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants