-
-
Notifications
You must be signed in to change notification settings - Fork 796
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
Drawer jitters on open when input has autofocus #693
Comments
It looks like this is happening due to Safari’s lack of support for preventScroll. This check appears to no longer work for some reason: https://github.com/shoelace-style/shoelace/blob/next/src/internal/support.ts I remember very specifically when this piece of code was added and it definitely was working. The reason it exists is so Shoelace can handle initial focus differently for Safari to prevent this exact problem. Perhaps the check is broken now because Safari added experimental support for this (I’m speculating, but I recall seeing a WebKit email about the feature recently so it wouldn’t surprise me.) This issue will go away on its own once Safari ships it, but until then we need a reliable check to work around it. I’ll see what I can come up with to feature detect preventScroll in a different way. |
There is a mention of this issue in this bug: |
I can repro this in https://next.shoelace.style/components/drawer?id=customizing-initial-focus I believe 946fcf2 resolved it. |
@claviska .. Tried this with my own project (using next) and it's still an issue with multiple different drawers. I am asynchronously rendering this using React so I wonder if there is again some timing issue ? |
Can you post a repro? |
I am using it with React + Scala.js so it's going to be a bit difficult to get a reproducible test case that makes sense. I have noticed that it only happens when:
I have my project running with a local version of Shoelace. Is there anything I can add to drawer.ts to aid debugging ? |
@claviska .. So have managed to track something down. If I disabled setInitialFocus within drawer.ts I noticed that the issue still appeared. If I removed the attribute from input.ts and let drawer.ts set the focus the issue was fixed. So would a fix be to remove the autofocus attributes from all focusable elements within drawer when it is first created ? |
If you remove I suspect it will not, and in that case I think it's worth making a minimal repro and filing a WebKit bug as requested here to call out the problem of autofocus + animations. Unfortunately, this is likely more of a Safari bug and if I can't feature detect it, I can't work around it. |
This looks similar to this as well: microsoft/reactxp#526 |
I have a workaround if you're interested. And it does make sense that you would want to disable the HTML Input autofocus behaviour when you're going to be doing it yourself. async show() {
if (this.open) {
return undefined;
}
this.open = true;
const target = this.querySelector('[autofocus]');
if (target) {
target.removeAttribute("autofocus");
target.setAttribute("initial-focus", "true");
}
return waitForEvent(this, 'sl-after-show');
}
private setInitialFocus() {
const target = this.querySelector('[initial-focus]');
if (target) {
(target as HTMLElement).focus({ preventScroll: true });
} else {
this.panel.focus({ preventScroll: true });
}
} |
It was easier to work around this than try to reduce it and file a Safari bug. The solution I used was similar to your suggestion. I'm removing Safari seems to be honoring |
Describe the bug
When a Drawer contains an input field with autofocus enabled then it will jitter on first open with Safari.
This could have been fixed with the animation frame but wanted to raise in case.
To Reproduce
Steps to reproduce the behavior:
Screenshots
Screen.Recording.2022-02-22.at.7.55.11.am.mov
Browser / OS
The text was updated successfully, but these errors were encountered: