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 error Snackbar to table components #505

Merged
merged 33 commits into from
Oct 23, 2023
Merged

Add error Snackbar to table components #505

merged 33 commits into from
Oct 23, 2023

Conversation

xplato
Copy link
Contributor

@xplato xplato commented Oct 2, 2023

Depends on #504 and #514

@xplato xplato marked this pull request as ready for review October 2, 2023 20:33
@razor-x
Copy link
Member

razor-x commented Oct 2, 2023

Any thoughts on how to trigger a loading error in a story? Maybe we need a workspace on the fake that always returns errors on get / list?

@vercel
Copy link

vercel bot commented Oct 2, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
react 🛑 Canceled (Inspect) Oct 3, 2023 5:17pm

@xplato
Copy link
Contributor Author

xplato commented Oct 2, 2023

Any thoughts on how to trigger a loading error in a story? Maybe we need a workspace on the fake that always returns errors on get / list?

@razor-x an erroring workspace isn't a bad idea. How would you go about that?

@razor-x
Copy link
Member

razor-x commented Oct 2, 2023

We can just add a DB method to the fake to configure some behavior, like db.simulateInternalServerError('/devices/list', { workspace_id })

@seveibar I know you have thought about this, did you ever have a particular API in mind?

@seveibar
Copy link
Contributor

seveibar commented Oct 2, 2023

@razor-x that pattern seems ok, although id always suggest documenting specific known bugs or scenarios when making simulation endpoints. In this case its not a bug we're replicating in the fake and the function is unlikely to cover up a bug (if it was repurposed) so i think its good!

@razor-x
Copy link
Member

razor-x commented Oct 2, 2023

That makes sense. In this case the error is a generic issue that could be caused by anything. But we could name it simulateTemporaryOutage following your suggestion @seveibar

Copy link
Member

@razor-x razor-x left a comment

Choose a reason for hiding this comment

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

We need a way to test this on storybook before we can merge it to make sure it renders correctly over the components.

@xplato
Copy link
Contributor Author

xplato commented Oct 23, 2023

@razor-x I believe this PR should be all unblocked now. Did some local testing and your recent fix seems to have patched up all the issues I had previously.

@xplato xplato merged commit a538770 into main Oct 23, 2023
@xplato xplato deleted the hook-up-snackbar branch October 23, 2023 16:42
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.

4 participants