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

Make the standalone instant launch page show errors immediately #610

Merged
merged 3 commits into from
Dec 9, 2024

Conversation

ianmcorvidae
Copy link
Member

This is better than putting it behind an ErrorTypographyWithDialog because there's not really much reason to just show a text error that tells you almost nothing with a detail button, on a page with nothing else. I also made it so if you close the error dialog it just redirects you to the dashboard since there's not much reason to stay on the page at that point.

@ianmcorvidae
Copy link
Member Author

Heh, yeah, this is a lot cleaner with WrappedErrorHandler.

errorObject={err}
/>
);
return <WrappedErrorHandler errorObject={error || resourceError} />;
Copy link
Member

Choose a reason for hiding this comment

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

This should also have a baseId, though I don't see one already setup for this button wrapper 🤔
I guess ids.BASE should work for now 🤷

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, I haven't been sure what to do for the debug IDs. I figure we don't really use them anymore really anyway, but I'll add... something, at least.

Copy link
Member

Choose a reason for hiding this comment

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

True, they were mainly for QA tests, and we haven't added any new RobotFramework/Selenium tests in a while, but all other uses of this component have a baseId, so I figure why not keep it consistent for now 🤷

Copy link
Member

@psarando psarando left a comment

Choose a reason for hiding this comment

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

LGTM 🎉 :shipit: 👍

@ianmcorvidae ianmcorvidae merged commit 1ef6170 into cyverse-de:main Dec 9, 2024
6 checks passed
@ianmcorvidae ianmcorvidae deleted the better-il-error branch December 9, 2024 18:59
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.

2 participants