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

Add update toast type when toast.promise resolves #851

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

alexandredev3
Copy link

@alexandredev3 alexandredev3 commented Oct 9, 2022

Now the type prop accepts a function and receives the data returned from the promise. It's similar to the render prop.

example:

const promise = () => Promise.resolve({ errors: 'error' });

toast.promise(promise, {
  success: {
    render: ({ data }) => data.errors ? 'Promise rejected' : 'Promise resolved',
    type: (data) => data.errors ? 'error' : 'success',
  },
  error: 'Promise rejected'
});

There's no need to work around using toast.update anymore if the promise resolves with an error.

resolves #819

@alexandredev3
Copy link
Author

Hey @fkhadra. Could you look at this PR and tell me what you think?

@alexandredev3 alexandredev3 changed the title Add update toast type when toast promise resolves Add update toast type when toast.promise resolves Oct 9, 2022
@coveralls
Copy link

Coverage Status

Coverage increased (+1.8%) to 92.422% when pulling 76689b6 on alexandredev3:feature/toast-promise-update-toast-type into 75525b1 on fkhadra:main.

@fkhadra
Copy link
Owner

fkhadra commented Oct 31, 2022

Hey @alexandredev3, as a graphql user I understand the need of adding such API. it can also be useful when using native fetch as it won't throw when the response is not ok.
That being said, I'm not fully convinced, your implementation is ok don't get me wrong 😊. I'm just not sure if this should belong to the library.
I'm doing a bunch of cleanups here #852. Let's keep the PR open for now and see what we can do about it when things will be more clear

@alexandredev3
Copy link
Author

alexandredev3 commented Nov 4, 2022

Hi @fkhadra! I don't fully understand why you're not sure if this should belong to the library.
To me, it makes sense to have such a feature in the library.
Similar to the render prop, I think we should also be able to change the toast type when the promise resolves. Since that would be really handy when dealing with APIs that won't throw an error.

@JavierBrooktec
Copy link

any updates on this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Have the "type" property accept a function that receives data when using toast.promise
4 participants