primefaces / primereact

The Most Complete React UI Component Library
https://primereact.org
MIT License
7k stars 1.06k forks source link

Dialog: onHide callback should be called when onExited callback is called. #6448

Closed didix16 closed 7 months ago

didix16 commented 7 months ago

Describe the bug

Hello there, I'm having some wierd behaviour (maybe missunderstanding the component, i don't know) with Dialog component. As I could see in examples docs and source code, when you make click on close icon, the onClose callback is called, calling at the same time onHide if it is defined. https://github.com/primefaces/primereact/blob/6ef3a903c887ff23d93c81ae49b76d7ece707de2/components/lib/dialog/Dialog.js#L69-L72

However, when you have to 'close' the dialog by some custom button in dialog footer, the only way to 'close' it is calling some method to make the property visible to false and then the dialog becomes hidden but there is no call to onHide dialog, which is wierd because I need to execute some logic when onHide is called but I need that logic to be executed when user press the footer button or closes the dialog. Yes, I know I can define a onHideCallback and set it to the footer button and the onHide prop of the dialog but then something happens. At some point, the callback is called twice in a row and I'm not getting the expected behavior that I need. image

By the way, I'm gonna explain what I'm trying to do and set a reporducer to stackbliz, demonstrating my issue.

So what I want to do is basically to show a dialog if the value of the Autocomplete is none of the list. The check process must be executed when the user hits 'Tab' or 'Enter' key inside the input of the Autocomplete or at onBlur callback from Autocomplete.

At the moment, the implementation of pressing Enter, works great. If you leave 'Enter' key pressed, it appears the Dialog, with button autofocus, so the dialog closes, returning the focus to Autocomplete and then the callback onKeyPress is executed again, showing the Dialog, and so on...

But the problem occurs when I want to implement the onBlur behavior. I need that when dialog closes, it must focus again on Autocomplete, forcing the user to select a value from the list. However, If I force the focus using .focus() method, the next time I leave pressed the 'Enter' key, the Dialog shows but neither without the modal effect (backdrop), nor the focus on the 'Ok' footer button which calls the setVisible(false) to hide the dialog.

So thus is why I'm trying to implement a workarround that only calls .focus() when the dialog is shown onBlur method and not when coming from keydown 'Enter' or 'Tab'. But again, as I said before, at some point the onHide callback its been called twice, and interrupting the functionality of my workarround.

Maybe executing onHide method from dialog prop only when the dialog become hidden it fixes the bug.

I see in code that onEntered callback calls the onShow callback https://github.com/primefaces/primereact/blob/6ef3a903c887ff23d93c81ae49b76d7ece707de2/components/lib/dialog/Dialog.js#L293-L301

but the onExited callback does not call the onHide callback

https://github.com/primefaces/primereact/blob/6ef3a903c887ff23d93c81ae49b76d7ece707de2/components/lib/dialog/Dialog.js#L309-L318

So, why not calling onHide inside onExited before focus the element, for example?

Sorry for my bad english. I hope the problem is understood

Reproducer

https://stackblitz.com/edit/qezure?file=src%2FApp.jsx

PrimeReact version

10.6.3

React version

18.x

Language

TypeScript

Build / Runtime

Vite

Browser(s)

No response

Steps to reproduce the behavior

  1. Type 'i' for example in autocomplete
  2. Then leave Enter key pressed
  3. Second time that Dialog shows, Dialog appears without backdrop and without focus.

Expected behavior

On Enter keypress without leaving it, it should keep open the dialog and then closing it indefinitely until the Enter key is released.

EDIT:

I just see this comment: https://github.com/primefaces/primereact/issues/880#issuecomment-494322143 referencing a similar issue with onHide callback.

I don't know why, if I add

if (!visible) return; // <== This should be added to work nicely!

at the top of handleDialogHide (which is the callback for onHide dialog property), then the behavior is the expected one.

But as I said before, I think we should use our custom logic without workarrounds. I think the custom logic should be called onHide callback even when a custom button changes the visibility of dialog to false (that is to say, when you want to close the dialog).

This is the new reproducer with the workarround that demonstrates the expected behaviour.

Reproducer

https://stackblitz.com/edit/qezure-z3uhd9?file=src%2FApp.jsx

didix16 commented 7 months ago

Thank you guys @melloware @Rekl0w

melloware commented 7 months ago

Looks like #6562 is now an issue which might need a tweak to this fix.

melloware commented 7 months ago

Reverting this fix as it broke a new scenario. @Rekl0w will explain.

didix16 commented 7 months ago

Mmm I just seen the #6562 issue. So at the moment I supose I have to do the workarround meantime. I'll wait @Rekl0w explanation. Thanks @melloware

Rekl0w commented 7 months ago

Simply, you have to do the workaround, it is the correct usage. Technically https://github.com/primefaces/primereact/issues/880#issuecomment-494322143 you need to use it like this code. For now, do the workaround. I will try to explain deeply if I can find free time. Thanks for the patience. @didix16

melloware commented 7 months ago

Yep according to PrimeTek your workaround is the correct workaround according to #880

didix16 commented 7 months ago

I see. Maybe some tips in Dialog documentation (i.e, include workaround in the examples and explain it) will help other people avoid searching workarounds/duplicate issues of same stuff.

Thanks for your work guys.

melloware commented 7 months ago

Agreed I just changed this to a Documentation ticket.