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

<sl-dialog> await hide() doesn't work in testing environment #2319

Open
BalusC opened this issue Dec 19, 2024 · 6 comments
Open

<sl-dialog> await hide() doesn't work in testing environment #2319

BalusC opened this issue Dec 19, 2024 · 6 comments
Labels
bug Things that aren't working right in the library.

Comments

@BalusC
Copy link

BalusC commented Dec 19, 2024

While using of Shoelace 2.17.1 I discovered 2 issues:

1. await hide() doesn't work before a second show()

dialog.show();
// ...
await dialog.hide();
dialog.show();

The second show basically reuses the same dialog component for different content. But it gets hidden immediately basically because the animated hide isn't fully completed at that moment. I'd intuitively expect the await hide() to hook on sl-after-hide. This is therefore open for improvement.

Update: Invalid. It was caused by a mistake on my side. Excuses! But the next issue still remains.

2. await hide() doesn't work in testing environment

The promise simply never resolves. I guess it's related to the headless nature of testing enrivonments. I'm using WTR with Chai/Mocha. This is probably also open for improvement. I've for now replaced it by a "sleep" of 150ms.

@BalusC BalusC added the bug Things that aren't working right in the library. label Dec 19, 2024
@KonnorRogers
Copy link
Collaborator

While using of Shoelace 2.17.1 I discovered 2 issues:

1. await hide() doesn't work before a second show()

dialog.show();
// ...
await dialog.hide();
dialog.show();

The second show basically reuses the same dialog component for different content. But it gets hidden immediately basically because the animated hide isn't fully completed at that moment. I'd intuitively expect the await hide() to hook on sl-after-hide. This is therefore open for improvement.

2. await hide() doesn't work in testing environment

The promise simply never resolves. I guess it's related to the headless nature of testing enrivonments. I'm using WTR with Chai/Mocha. This is probably also open for improvement. I've for now replaced it by a "sleep" of 150ms.

I belive show() is also a promise. Could you try:

await dialog.show();
await dialog.hide();
await dialog.show();

@BalusC BalusC changed the title <sl-dialog> await hide() doesn't allow a second show() and also doesn't work in testing environment <sl-dialog> await hide() doesn't work in testing environment Dec 20, 2024
@BalusC
Copy link
Author

BalusC commented Dec 20, 2024

While creating a reproducer I figured out the await hide() before show actually works fine. Excuses!

The second issue still remains though. The await hide() never resolves in a testing environment.

@CetinSert
Copy link

CetinSert commented Dec 20, 2024

Playground with multiple versions:

  1. Open sl.rt.ht/2319 → ✔️ 2.19.1 everything works! (Press F12 to confirm!)
  2. Press Ctrl + I to focus the input panel.
  3. Press × 2 → ✔️ 2.17.1 everything works! (Press F12 to confirm!)

@BalusC – cannot reproduce the issue!

image

@BalusC
Copy link
Author

BalusC commented Dec 20, 2024

The keyword is "testing environment". E.g. WTR/Mocha/Chai.

@KonnorRogers
Copy link
Collaborator

The keyword is "testing environment". E.g. WTR/Mocha/Chai.

What testing environment? Do you have a reproduction we could look at?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Things that aren't working right in the library.
Projects
None yet
Development

No branches or pull requests

4 participants
@CetinSert @BalusC @KonnorRogers and others