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

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

Open
wants to merge 6 commits into
base: main
Choose a base branch
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

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants