KingSora / OverlayScrollbars

A javascript scrollbar plugin that hides the native scrollbars, provides custom styleable overlay scrollbars, and preserves the native functionality and feel.
https://kingsora.github.io/OverlayScrollbars
MIT License
3.83k stars 214 forks source link

The 'blur' event fires when the input element with 'autoFocus' set to 'true' is wrapped with 'OverlayScrollbars'. #605

Closed thalaz closed 6 months ago

thalaz commented 8 months ago

Describe the bug

If we take a regular <input /> element, set its autoFocus to true, and wrap it with the OverlayScrollbars - an additional blur event will fire on the first render.

Reproduces in Chrome only.

To Reproduce

Steps to reproduce the behavior:

  1. Open Chrome browser
  2. Go to With OverlayScrollbarsComponent or With useOverlayScrollbars
  3. Click the refresh button provided by the StackBlitz
  4. Check the Console - you should see one onBlur message

Expected behavior

There is no blur event on the initial render. The blur event should fire only on the actual blur action (clicking outside of the input).

Examples

Environment

Additional context

KingSora commented 8 months ago

@thalaz Good day :)

This happens because OverlayScrollbars wraps your elements and thus causes the original element to lose focus (because its briefly removed / moved in the DOM). After the wrapping is done it re-focuses the originally focused element so the original state is restored.

May I ask what you're trying to achieve that this behavior is unwanted?

thalaz commented 8 months ago

@KingSora, good day :)

I'll provide you with an example of the actual case shortly.

thalaz commented 8 months ago

@KingSora, here is an example.

On my project, we have a lot of modals. They differ in content and size, and since users can have different devices with different dimensions, we wanted to add a custom scrollbar to improve the UX.

We use the MUI library for styling and react-hook-form library to handle forms.

As for the product requirements, we need to have an instant validation on the fields which is achieved with mode: 'all' in the example provided. Also, I've extended the DialogContent as you may see in the CustomDialogContent component - the only scrollable area will be the actual content of the modal. And we would like to have the first field auto-focused, which is also required in some/most of the cases.

In the example:

thalaz commented 8 months ago

From your words, I also understand that it's the desired behaviour (so it's not a bug, but rather an improvement suggestion).

Any suggestions on how I can keep both the scrollbar and the auto-focus functionality and get rid of this behaviour?

KingSora commented 8 months ago

@thalaz I've created an workaround example: https://stackblitz.com/edit/bug-report-9d6408-usage-example-hjz9u9?file=src%2FDialogExample%2FCustomDialogContent%2Findex.tsx

I believe I'll add something like this to the library as well.. But for now please use the workaround until I've decided how this issue should be handled

thalaz commented 8 months ago

@KingSora, thanks for a workaround. It was working only on the first modal open, but I see that you updated the workaround and it seems like everything is fine now.

Waiting for the official fix :)

thalaz commented 8 months ago

@KingSora, it doesn't work in the StrictMode though (I guess it's caused by the additional mount/unmount).

I have used HTML's dialog since MUI's dialog has it's own issue with the StrictMode (it loses the focus completely).

It's not a big deal for me, but just FYI.

KingSora commented 8 months ago

@thalaz Thanks for the feedback :)

I've revisited the problem and noticed that the functionality is easier and more reliable to implement with a Plugin. Here is the code: https://stackblitz.com/edit/bug-report-9d6408-usage-example-strict-mode-7mb27t?devToolsHeight=33&file=src%2Findex.tsx

This should also work with StrictMode I believe

thalaz commented 8 months ago

Thanks, @KingSora. That's a very good and tidy one. Plus will work for all usages "out of the box".

Tested in my project - works as well.

Should I consider this as a final solution and forget about this? Or will this become a part of the overlayscrollbars package someday?

KingSora commented 8 months ago

@thalaz I'll probably add it to the library in the next version.. I'll let you know when its released :)

thalaz commented 8 months ago

Thanks a lot, @KingSora!

KingSora commented 8 months ago

@thalaz I've published overlayscrollbars v2.5.0 which includes a fix to this issue :) Please check it out and give feedback whether it makes the workaround unnecessary

thalaz commented 8 months ago

@KingSora, unfortunately, updating the library doesn't help 🙁 A workaround is still needed.

In this example, I have commented the OverlayScrollbars.plugin(blurPlugin); and StrictMode - behaviour is still reproducible in "Open modal with OverlayScrollbars" (red field when you open the modal)

Here is a simple example (without MUI, etc.): link (instant onBlur message in the Console).

Note: tested in StackBlitz examples and my project.

KingSora commented 8 months ago

@thalaz Thanks for the feedback! After revisiting the code which was supposed to fix the issue I've noticed that I'm only covering the focus and blur events directly, but forgot the focusin and focusout events.. I've updated the code and tests accordingly: https://github.com/KingSora/OverlayScrollbars/commit/ec9ca7c9be8e49a6d975fb1e4fb2b049c86b9fce

I'll release a "fixed fixed" version soon-ish... Sorry for the mistake, for the time being please continue to use the workaround

thalaz commented 8 months ago

Yes sure @KingSora, no problem at all 🙂

Please let me know when a fixed version is deployed, and I'll double-check it 🙂

KingSora commented 7 months ago

@thalaz I've published overlayscrollbars v2.6.0 which should now hopefully have the correct fix :)

thalaz commented 6 months ago

Hi @KingSora,

Thank you very much.

Tested on the previous examples and real project.

It is fixed now. 🎉