-
Notifications
You must be signed in to change notification settings - Fork 3
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
19655 adds reusable snackbar component to all project view tables #1521
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for seeing this opportunity to clean up our feedback messages while working on the table feedback. I know that we haven't been the best about keeping this consistent as we added features over the years, and I think these updates will change that for the better in this main view.
I requested some small changes in here, and I think it is 🚢 from there. I was able to see the feedback in all places noted in the test steps. 💯
While testing, I noticed an old bug that isn't introduced in this PR with how we are closing the snackbar with setTimeout. I'll write up a separate issue.
moped-editor/src/views/projects/projectView/ProjectWorkActivity/ProjectWorkActivityForm.js
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have tested this and tried to break it, and I cant! I will look again after the requested changes are addressed and approve then. its a lot of work and you were very thorough
thanks @mddilley !! i agree with all of your edits and i'll get those incorporated shortly. |
i noticed this bug as well! once we have the snackbar defined in a single component it should make this an easier fix and hopefully prevent it from reappearing in the future |
yay, thank you!! i'm glad it stood up to the test :) |
i chatted with Tilly and moved a couple changes from this PR into cityofaustin/atd-data-tech#20772 so we can review and release this one today. I'll start reviewing shortly. 👀 |
snackbarState, | ||
snackbarHandle, | ||
handleSnackbar, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
much better name! thank you
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I ran through the test steps again and i like the updated messages to refresh the page. this is a slick upgrade imo!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great! I reviewed the latest commits, and I verified the feedback is working in the project summary and tables. Thanks for updating! 🙌 🚀 🙌
I found one tiny itsy bitsy thing, when you delete a file while offline it will actually disappear and look like it was deleted despite giving the error. If you go back online and refresh the page it will be there though |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this looks and works so well, it was such a huge undertaking and improvement! awesome to have these consistent feedback snackbars throughout the app. i found one weird behavior with the file deleting but other than that this is good to go!!
@tillyw @chiaberry Heads up that I resolved a small conflict with @roseeichelmann Thanks for finding that bug! Maybe there is a state update that happens before the mutation fails. I'll write up an issue! 📝 🙏 |
thanks @mddilley for resolving that conflict! @roseeichelmann thanks for catching that! i noticed that behavior as well, and i think it would be worth addressing in a follow-up issue. thanks mike for making an issue for that as well! |
Associated issues
fixes cityofaustin/atd-data-tech#19655
Testing
URL to test:
https://deploy-preview-1521--atd-moped-main.netlify.app/ or local
Steps to test:
this pr adds snackbar error and success messages to all of the mutations in the project view. i tried to keep all the error messages and functionality as consistent as possible.
please let me know if i have missed anything!
Ship list