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 alert for failed requests #1018

Merged
merged 7 commits into from
Nov 10, 2023
Merged

Conversation

andchiind
Copy link
Contributor

Closes #1008

@andchiind andchiind added improvement Improvement to existing functionality frontend Frontend related functionality labels Sep 21, 2023
@andchiind andchiind self-assigned this Sep 21, 2023
@andchiind andchiind changed the title Move alerts to header component Add alert for failed requests Sep 21, 2023
@andchiind andchiind force-pushed the 1008-error-banner branch 3 times, most recently from 756ac79 to 03b52ba Compare September 22, 2023 08:13
@andchiind andchiind marked this pull request as ready for review September 22, 2023 08:14
@andchiind
Copy link
Contributor Author

Ideally I would like the banner alert to be triggered in ApiCaller.tsx, so that it's integrated in the event handler. Then we could neatly decide how each request should be handled in a single file. This is difficult though since BackendAPICaller is a static class which does not have access to custom contexts, and so we therefore need to update the banner in .catch statements where the BackendAPICaller is used. It would be possible to implement this custom error handler in ApiCaller.tsx if we made the custom class into a custom context instead though.

@andchiind
Copy link
Contributor Author

As a part of implementing the alert in such a way that the dialog closes itself when an error is returned, I ended up refactoring MissionQueueView.tsx and the dialog component file.

@andchiind andchiind force-pushed the 1008-error-banner branch 3 times, most recently from f08dd9d to 6d16769 Compare October 2, 2023 13:09
@Eddasol
Copy link
Contributor

Eddasol commented Oct 10, 2023

image
image
Gap between "Add mission" and "Create mission" changes when "Add mission" is pressed

@Eddasol
Copy link
Contributor

Eddasol commented Oct 10, 2023

image
Suggest updating style in "Request error" to match existing style for failed mission

@andchiind andchiind force-pushed the 1008-error-banner branch 2 times, most recently from 0155a57 to 47d975e Compare October 12, 2023 10:45
@andchiind
Copy link
Contributor Author

image Suggest updating style in "Request error" to match existing style for failed mission

I've now made them as similar as possible without changing the information.

@andchiind
Copy link
Contributor Author

image image Gap between "Add mission" and "Create mission" changes when "Add mission" is pressed

This should be fixed now.

@andchiind andchiind force-pushed the 1008-error-banner branch 2 times, most recently from a8ef412 to 81a525e Compare October 30, 2023 15:16
@andchiind andchiind force-pushed the 1008-error-banner branch 2 times, most recently from d8b652a to 24822ec Compare November 9, 2023 08:52
@andchiind andchiind force-pushed the 1008-error-banner branch 2 times, most recently from 4e4bded to 8125894 Compare November 10, 2023 10:59
@mrica-equinor mrica-equinor force-pushed the 1008-error-banner branch 2 times, most recently from fee2eb2 to 4c681df Compare November 10, 2023 12:17
Copy link
Contributor

@mrica-equinor mrica-equinor left a comment

Choose a reason for hiding this comment

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

LGTM :)

@andchiind andchiind merged commit 7c44dcc into equinor:main Nov 10, 2023
7 checks passed
@andchiind andchiind deleted the 1008-error-banner branch November 10, 2023 13:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
frontend Frontend related functionality improvement Improvement to existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve error handling on fetching echo missions
3 participants