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

fix(app): add an exit button for failed moveToAddressable area commands during Error Recovery #16729

Merged
merged 2 commits into from
Nov 7, 2024

Conversation

mjhuff
Copy link
Contributor

@mjhuff mjhuff commented Nov 7, 2024

Closes RQA-3542

Overview

See the ticket for the steps to reproduce. The moveToAddressableArea command did not have correct conditional logic in some spots for handling the error if it occurred during a fixit flow. This PR refactors the command logic, so any error will be handled correctly if the flow is fixit.

Also, recent fixes for unrendering pipette cards made it so that errors which occur during drop tip wizard during error recovery would automatically cancel the run instead of showing users the proper "error" modal, so that is fixed as well.

Current Behavior

Screenshot 2024-11-07 at 2 25 41 PM

Fixed Behavior

Screenshot 2024-11-07 at 4 04 18 PM

Test Plan and Hands on Testing

  • See tickets for steps to reproduce.
  • Verified the button exists (see picture), and the flows don't automatically cancel the run after the error occurs (also see picture).

Changelog

  • Fixed a bug during error recovery in which certain errors during drop tip wizard flows did not render an escape button.

Risk assessment

low

@mjhuff mjhuff requested a review from a team November 7, 2024 21:32
@mjhuff mjhuff requested a review from a team as a code owner November 7, 2024 21:32
@mjhuff mjhuff requested review from ncdiehl11 and removed request for a team and ncdiehl11 November 7, 2024 21:32
Comment on lines +46 to +50
setErrorDetails({
header: header ?? t('cant_safely_drop_tips'),
message: messageText ?? t('remove_the_tips_manually'),
type,
})
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added some fallback copy just in case.

@ecormany
Copy link
Contributor

ecormany commented Nov 7, 2024

"Return to menu" struck me as a little odd, but I see it's already in our localization strings, so carry on.

@mjhuff mjhuff force-pushed the app_add-drop-tip-exit-btn-recovery branch from ce3664c to 5b40f90 Compare November 7, 2024 21:41
@smb2268 smb2268 self-requested a review November 7, 2024 22:14
Copy link
Contributor

@smb2268 smb2268 left a comment

Choose a reason for hiding this comment

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

Not able to test this one remotely but the code changes seem sound 🤘🏻

@mjhuff mjhuff merged commit d4c0f85 into chore_release-8.2.0 Nov 7, 2024
30 checks passed
@mjhuff mjhuff deleted the app_add-drop-tip-exit-btn-recovery branch November 7, 2024 22:25
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.

3 participants