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

feat(protocol-designer): show confirm modal before losing form's unsaved changes #6040

Merged
merged 4 commits into from
Jul 2, 2020

Conversation

IanLondon
Copy link
Contributor

@IanLondon IanLondon commented Jul 1, 2020

Overview

Closes #5472

Also closes #6042, a bug the E2E tests accidentally uncovered due to this feature exposing whether a form has any changes (in a test where a Magnet form shouldn't have any changes, there were changes due to this bug, which failed the E2E tests!)

Changelog

Review requests

  • Cancelling a previously-saved step form should prompt for confirmation, only if there are changes. This should occur for all 4 methods of cancelling a step form:

  • cancel by clicking Cancel button

  • cancel by clicking another step item

  • cancel by clicking a terminal item

  • cancel by creating a new step

  • If the form doesn't really have changes (either no interaction happened since opening, or whatever you changed you then changed back), then all 4 methods above should not show the confirm modal

  • Deleting a step, with changes or without, should show a modal with unique copy "You have not saved this step form. If you navigate away without saving, this step will be deleted."

  • bug: unsaved form, if open, remembers deleted magnetic module ID #6042 should be fixed (see ticket)

Risk assessment

Medium, PD-only

@IanLondon IanLondon added ready for review protocol designer Affects the `protocol-designer` project labels Jul 1, 2020
@IanLondon IanLondon requested review from Kadee80, shlokamin and a team July 1, 2020 15:58
Copy link
Contributor

@Kadee80 Kadee80 left a comment

Choose a reason for hiding this comment

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

🌵

Copy link
Member

@shlokamin shlokamin left a comment

Choose a reason for hiding this comment

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

Nice! But shouldn't e2e tests now be passing?

@codecov
Copy link

codecov bot commented Jul 2, 2020

Codecov Report

❗ No coverage uploaded for pull request base (edge@11dc15b). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             edge    #6040   +/-   ##
=======================================
  Coverage        ?   79.22%           
=======================================
  Files           ?      187           
  Lines           ?    18127           
  Branches        ?        0           
=======================================
  Hits            ?    14362           
  Misses          ?     3765           
  Partials        ?        0           
Impacted Files Coverage Δ
opentrons/hardware_control/modules/update.py 28.12% <0.00%> (ø)
...er/robot_server/robot/calibration/check/session.py 83.22% <0.00%> (ø)
opentrons/system/wifi.py 93.61% <0.00%> (ø)
opentrons/hardware_control/util.py 91.66% <0.00%> (ø)
opentrons/system/smoothie_update.py 50.00% <0.00%> (ø)
opentrons/hardware_control/modules/types.py 95.83% <0.00%> (ø)
opentrons/hardware_control/types.py 92.22% <0.00%> (ø)
opentrons/hardware_control/controller.py 73.80% <0.00%> (ø)
opentrons/legacy_api/robot/robot.py 84.81% <0.00%> (ø)
opentrons/legacy_api/modules/tempdeck.py 71.87% <0.00%> (ø)
... and 177 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 11dc15b...9b50a93. Read the comment docs.

Copy link
Member

@shlokamin shlokamin left a comment

Choose a reason for hiding this comment

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

🚢

@IanLondon IanLondon merged commit e32cc01 into edge Jul 2, 2020
@IanLondon IanLondon deleted the pd_confirm-unsaved-form-changes-5472 branch July 2, 2020 17:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
protocol designer Affects the `protocol-designer` project
Projects
None yet
3 participants