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

Fixes Edge bug with inputs and textareas #22

Merged
merged 2 commits into from Sep 5, 2020

Conversation

kennethormandy
Copy link
Contributor

I happened to see some inconsistent results with this polyfill on Edge v18.

I am doing some very light user agent checking to resolve this, which is obviously not ideal, but I couldn’t find another way to solve the problem since Edge does partially support focusOptions.

There’s also a tradeoff in that the polyfill is then used for everything, so you get a brief “flash” when the scrolling is prevented. That part is similar to what we saw on Safari, because it also needs the setTimeout.

Let me know what you think of this solution!

Without the polyfill

Doesn’t work the text inputs, but works natively on the button.

edge18-without-fix

With the polyfill, and this fix

Works on both.

edge18-with-fix-both

@calvellido calvellido self-requested a review August 11, 2020 16:28
@calvellido
Copy link
Owner

Interesting! Thanks for the further review on Edge @kennethormandy! And also for adding the historic version of the Edge issue to the README 🙌.

Well, it's a sad thing that our friend Edge lies 😅. With that being the case, most probably there won't be any other option than to check for the UA. While everything is alright, if you are OK with it, I'm gonna leave the PR open to have a look at it at the weekend, where I'm gonna be able to explore this a bit more.

The flash bit is really tiring, but I'm not sure something additional could be done to prevent it 🤔. Also, it's probably more prone to happen on low-end devices, or bound to happen on computationally stressed instances, as I have had different results when testing.

Thanks again!

@kennethormandy
Copy link
Contributor Author

While everything is alright, if you are OK with it, I'm gonna leave the PR open to have a look at it at the weekend, where I'm gonna be able to explore this a bit more.

Sure, feel free to take your time with it. I’ll use my fork as needed in the meantime. Thanks for the review.

@calvellido calvellido merged commit 86a6927 into calvellido:master Sep 5, 2020
Copy link
Owner

@calvellido calvellido left a comment

Choose a reason for hiding this comment

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

👍

@calvellido calvellido added the bug Something isn't working label Sep 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants