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

Toaster: make closeAll function async to allow awaiting it #1025

Closed
wants to merge 4 commits into from

Conversation

mayank99
Copy link
Contributor

@mayank99 mayank99 commented Jan 23, 2023

Changes

Instead of hoping that promises resolve correctly, users can now await close calls to our toaster before opening other toasts.

(async () => {
  await toaster.closeAll();
  toaster.informational();
})();

Does it fix #1005? Does it break anything? Lets find out!!

Edit: It does break things. I'll figure out a different approach. Stay tuned.

Edit 2: Fixed failing tests by not awaiting creation functions. See #1025 (comment).

Testing

Pending.

Docs

Pending

@mayank99 mayank99 changed the title Make toaster functions async to allow awaiting them Make toaster close functions async to allow awaiting them Jan 23, 2023
Comment on lines +143 to 146
// cannot `await` this, for backwards compat with the return value
this.updateView();

return { close: () => this.closeToast(currentId) };
Copy link
Contributor Author

@mayank99 mayank99 Jan 23, 2023

Choose a reason for hiding this comment

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

i wanted to make all methods awaitable, but that would cause breaking changes for users who are using the return value like this:

const { close } = toaster.positive();

i thought about keeping backwards compatibility by adding a promise as a new property in the return value.

Suggested change
// cannot `await` this, for backwards compat with the return value
this.updateView();
return { close: () => this.closeToast(currentId) };
const close = () => this.closeToast(currentId);
const updatePromise = this.updateView();
const done = new Promise<boolean>(async (resolve) => {
await updatePromise;
resolve(true);
});
return { close, done };

but no one has asked for it and i don't see the benefit yet, so i decided against it.

@mayank99 mayank99 changed the title Make toaster close functions async to allow awaiting them Toaster: make closeAll function async to allow awaiting it Jan 23, 2023
@mayank99 mayank99 force-pushed the mayank/async-toaster branch from d7ab0d3 to 4906822 Compare January 24, 2023 15:14
@mayank99 mayank99 closed this Jan 25, 2023
@mayank99 mayank99 deleted the mayank/async-toaster branch March 8, 2023 18:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Toast is not shown during first trigger
1 participant