Added Toast to Update User Profile Form#820
Conversation
…styling of .app Toast Container
|
@hawkishpolicy is attempting to deploy a commit to the openbeta-dev Team on Vercel. A member of the Team first needs to authorize it. |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
| setProfile(profile) | ||
| // Set the flag that indicates the user just submitted a change | ||
| setJustSubmitted(true) | ||
| // Removed old state for Snackbar |
There was a problem hiding this comment.
Can you remove this reference to the Snackbar component? I don't think it's useful to know it used to be here.
|
Thanks for the contribution. Just a few verbage changes I would suggest. If leaving a comment, it might be more useful to say what it does instead of "Added..[thing]", "Removed..[thing]". |
|
@hawkishpolicy Sorry to go back on this, but I'm not sure how much those comments are adding, the things you wrote seem to pretty self explanatory and I would do minimal comments at most. Also, we may change the behavior in the future to move to the toast to a different position, so telling us the position of it currently might not make sense. We could just go find where we defined this toast initially to see what options it has. Also, can you check to make sure the Snackbar.tsx component is no longer used anywhere else, and then remove it and all references to it. We can use Again, thanks for taking this on! Lemme know if you've got any other questions! |
|
I'm happy to change it. I'm brand new. Sorry for the kinks.
…On Thu, May 11, 2023, 12:12 PM Clinton Lunn ***@***.***> wrote:
@hawkishpolicy <https://github.com/hawkishpolicy> Sorry to go back on
this, but I'm not sure how much those comments are adding, the things you
wrote seem to pretty self explanatory and I would do minimal comments at
most. Also, we may change the behavior in the future to move to the toast
to a different position, so telling us the position of it currently might
not make sense. We could just go find where we defined this toast initially
to see what options it has.
Also, can you check to make sure the Snackbar.tsx component is no longer
used anywhere else, and then remove it and all references to it. We can use
react-toastify completely in its place.
Again, thanks for taking this on! Lemme know if you've got any other
questions!
—
Reply to this email directly, view it on GitHub
<#820 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/A5LHYZ3AVGQOINSSPBBVZOLXFUFYJANCNFSM6AAAAAAX5FOB2E>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
So can I just remove all comments and remove snackbar if it's no longer
being used elsewhere? that wasn't in the original bug post so i didn't want
to overstep.
I can do comments however you need them. It's unfortunately not very
intuitive to me about which comments are necessary and which are not.
Thanks for the guidance.
On Thu, May 11, 2023 at 12:54 PM Thomas Johnson ***@***.***>
wrote:
… I'm happy to change it. I'm brand new. Sorry for the kinks.
On Thu, May 11, 2023, 12:12 PM Clinton Lunn ***@***.***>
wrote:
> @hawkishpolicy <https://github.com/hawkishpolicy> Sorry to go back on
> this, but I'm not sure how much those comments are adding, the things you
> wrote seem to pretty self explanatory and I would do minimal comments at
> most. Also, we may change the behavior in the future to move to the toast
> to a different position, so telling us the position of it currently might
> not make sense. We could just go find where we defined this toast initially
> to see what options it has.
>
> Also, can you check to make sure the Snackbar.tsx component is no longer
> used anywhere else, and then remove it and all references to it. We can use
> react-toastify completely in its place.
>
> Again, thanks for taking this on! Lemme know if you've got any other
> questions!
>
> —
> Reply to this email directly, view it on GitHub
> <#820 (comment)>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/A5LHYZ3AVGQOINSSPBBVZOLXFUFYJANCNFSM6AAAAAAX5FOB2E>
> .
> You are receiving this because you were mentioned.Message ID:
> ***@***.***>
>
|
|
@hawkishpolicy You're right. It was a little bit late when I wrote up that issue 😅 when I said
I meant removing Snackbar component entirely because it's not needed anymore, BUT I neglected to say that more explicitly. These changes look good to me. @vnugent good to merge? |
|
@all-contributors add @hawkishpolicy for code |
|
I've put up a pull request to add @hawkishpolicy! 🎉 |
3 participants
Removed old Snackbar component.
Removed Snackbar div
Removed old justSubmitted and setJustSubmitted state
Added Toast.success for successful profile update
Added Toast.error for update failure
updated psuedo-code