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(app): allow custom labware dir to be opened and reset to default #4918

Merged
merged 6 commits into from
Feb 7, 2020

Conversation

mcous
Copy link
Contributor

@mcous mcous commented Feb 4, 2020

overview

This PR adds CTA's to the custom labware source directory UI to allow the user to:

  • Open the directory on their computer
  • Change the directory to another folder
  • Reset the directory to its default

Closes #4878, closes #4879

changelog

  • feat(app): allow custom labware dir to be opened and reset to default

review requests

ACs copied from tickets

reset directory

  • User able to click a link (or button) to reset source directory to default file path
  • User prompted to confirm or cancel change
  • Labware files are not moved

open directory

  • User able to click a link (or button) to open source folder in OS file browser

@mcous mcous added feature Ticket is a feature request / PR introduces a feature app Affects the `app` project ready for review robot-svcs Falls under the purview of the Robot Services squad (formerly CPX, Core Platform Experience). labels Feb 4, 2020
@mcous mcous requested review from iansolano and sanni-t February 4, 2020 21:57
@codecov
Copy link

codecov bot commented Feb 4, 2020

Codecov Report

Merging #4918 into edge will decrease coverage by 0.26%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             edge    #4918      +/-   ##
==========================================
- Coverage   67.59%   67.33%   -0.27%     
==========================================
  Files        1056     1065       +9     
  Lines       35499    36266     +767     
==========================================
+ Hits        23997    24419     +422     
- Misses      11502    11847     +345     
Impacted Files Coverage Δ
...p-generation/getNextRobotStateAndWarnings/index.js 59.32% <0.00%> (-3.18%) ⬇️
...i/tests/opentrons/hardware_control/test_modules.py 69.60% <0.00%> (-3.12%) ⬇️
opentrons/hardware_control/modules/__init__.py 50.00% <0.00%> (-2.64%) ⬇️
...ocol-designer/src/step-generation/errorCreators.js 91.66% <0.00%> (-2.09%) ⬇️
...-library/src/labware-creator/labwareDefToFields.js 87.09% <0.00%> (-1.80%) ⬇️
app/src/robot-api/reducer.js 88.23% <0.00%> (-1.77%) ⬇️
opentrons/server/endpoints/update.py 22.80% <0.00%> (-1.59%) ⬇️
opentrons/main.py 34.50% <0.00%> (-0.11%) ⬇️
app/src/modules/actions.js 100.00% <0.00%> (ø) ⬆️
shared-data/js/constants.js 100.00% <0.00%> (ø) ⬆️
... and 41 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 00b8d5b...8b494a6. Read the comment docs.

Copy link
Contributor

@iansolano iansolano left a comment

Choose a reason for hiding this comment

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

LGTM, mostly really small comments. 🌮

app-shell/src/config.js Outdated Show resolved Hide resolved
]}
>
<p>{CLICK_RESET_SOURCE_TO_RESET_YOUR_CUSTOM_LABWARE_DIRECTORY}</p>
<p>{LABWARE_FILES_IN_YOUR_CURRENT_DIRECTORY_WILL_NOT_BE_MOVED}</p>
Copy link
Contributor

Choose a reason for hiding this comment

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

very explicit 😂

const addFailure = useSelector(getAddLabwareFailure)
const labwarePath = config.labware.directory
const handleOpenPath = () => dispatch(openCustomLabwareDirectory())
const handleResetPath = () => dispatch(resetCustomLabwareDirectory())
const handleChangePath = () => dispatch(changeCustomLabwareDirectory())
const handleAddLabware = () => dispatch(addCustomLabware())
Copy link
Contributor

Choose a reason for hiding this comment

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

obvi not for in this PR but I wonder if prefixing with dispatch makes more sense instead of handle

app/src/components/IconCta/__tests__/IconCta.test.js Outdated Show resolved Hide resolved
app/src/config/index.js Outdated Show resolved Hide resolved
Copy link
Contributor

@iansolano iansolano left a comment

Choose a reason for hiding this comment

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

Nice! Looks really good.

@@ -0,0 +1,65 @@
// @flow
Copy link
Contributor

Choose a reason for hiding this comment

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

nice!

Copy link
Member

@sanni-t sanni-t left a comment

Choose a reason for hiding this comment

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

✅ Tested on macOS

@mcous mcous merged commit 03c438a into edge Feb 7, 2020
@mcous mcous deleted the app_open-custom-labware-dir branch February 7, 2020 22:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
app Affects the `app` project feature Ticket is a feature request / PR introduces a feature robot-svcs Falls under the purview of the Robot Services squad (formerly CPX, Core Platform Experience).
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add Labware: Open labware directory Add Labware: Reset labware directory
3 participants