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.

Sign up for GitHub

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

Jump to bottom

feat(app): populate ChooseRobotSlideout with runtime parameters #14706

Merged
merged 10 commits into from
Mar 25, 2024
Merged

feat(app): populate ChooseRobotSlideout with runtime parameters #14706

merged 10 commits into from
Mar 25, 2024

Conversation

ncdiehl11
Copy link
Collaborator

@ncdiehl11 ncdiehl11 commented Mar 20, 2024 •
edited
Loading

closes AUTH-98
closes AUTH-99
closes AUTH-101

Overview

Populate second page of ChooseRobotSlideout with RTPs.
Add link for restoring values to default.
Save state for in an object that will be used in createRunFromProtocolSource

Still todo:

  • error handling for numeric input fields
  • sending updated RTP object along with srcFiles during protocol run creation

Test Plan

  • select any protocol (this PR uses mock data until analysis provides RTP)
  • select Start setup button to open ChooseRobot slideout
  • select any robot and 'Continue to parameters' footer button
  • observe toggle, numerical input fields, and dropdown menus set to their default values (NOTE: dropdown fields will initialize with the first choice whose value matches its RTP's default value, regardless of its position in the array of choices)
  • observe tooltip on disabled 'Restore default values' link when all values are default
  • observe that all values are restored to default after any RTPs are changed and 'Restore default values' link is clicked

Changelog

Review requests

authorship stakeholders

Risk assessment

ncdiehl added 5 commits March 20, 2024 15:30
@ncdiehl
feat(app): populate chooserobotslideout with runtime parameters
fc72fa9 
Populate second page of ChooseRobotSlideout with RTPs. Add link for restoring values to default.
Save state for in an object that will be used in createRunFromProtocolSource
@ncdiehl
replace SelectField with DropdownMenu
9b13706
@ncdiehl
conditionally enable restore defaults link
e030a1d
@ncdiehl
add translation
357df9c
@ncdiehl
handle no RTPs being passed to chooserobotslideout
dd223ba
@ncdiehl11 ncdiehl11 self-assigned this Mar 20, 2024
@ncdiehl11 ncdiehl11 requested review from koji, shlokamin and jerader March 20, 2024 20:55
@ncdiehl11 ncdiehl11 marked this pull request as ready for review March 20, 2024 20:55
@ncdiehl11 ncdiehl11 requested a review from a team as a code owner March 20, 2024 20:55
@ncdiehl11 ncdiehl11 changed the title feat(app): populate chooserobotslideout with runtime parameters feat(app): populate ChooseRobotSlideout with runtime parameters Mar 20, 2024
jerader
jerader reviewed Mar 21, 2024
View reviewed changes
Copy link
Collaborator

@jerader jerader left a comment

Choose a reason for hiding this comment

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

this is looking awesome, good work! I left some cleanup suggestions

@koji
Copy link
Contributor

koji commented Mar 21, 2024

Could you update the tests?

@ncdiehl11 ncdiehl11 requested review from koji and jerader March 21, 2024 19:11
@ncdiehl11
Copy link
Collaborator Author

@koji @jerader Thank you guys so much for the detailed reviews! I have made the changes you suggested/commented where appropriate. I will push one more commit for adding test coverage for slideouts passing and rendering RTPs and then can merge once approved.

koji
koji reviewed Mar 21, 2024
View reviewed changes
Copy link
Contributor

@koji koji left a comment

Choose a reason for hiding this comment

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

left a couple of comments, but the changes look good to me.
nice work!

app/src/organisms/ChooseRobotSlideout/index.tsx Show resolved Hide resolved
app/src/organisms/ChooseRobotSlideout/index.tsx Show resolved Hide resolved
app/src/organisms/ChooseRobotSlideout/index.tsx Show resolved Hide resolved
jerader
jerader approved these changes Mar 25, 2024
View reviewed changes
Copy link
Collaborator

@jerader jerader left a comment

Choose a reason for hiding this comment

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

lgtm! After Koji's comments are addressed this is good to merge!

@ncdiehl11 ncdiehl11 merged commit 40236b5 into edge Mar 25, 2024
38 checks passed
@ncdiehl11 ncdiehl11 deleted the feat_app-RTP-slideout branch March 25, 2024 18:17
Carlos-fernandez pushed a commit that referenced this pull request May 20, 2024
@ncdiehl11 @Carlos-fernandez
feat(app): populate ChooseRobotSlideout with runtime parameters (#14706)
c4cb035 
closes [AUTH-98](https://opentrons.atlassian.net/browse/AUTH-98)
closes [AUTH-99](https://opentrons.atlassian.net/browse/AUTH-101)
closes [AUTH-101](https://opentrons.atlassian.net/browse/AUTH-101)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@koji koji koji left review comments

@jerader jerader jerader approved these changes

@shlokamin shlokamin Awaiting requested review from shlokamin

Assignees

@ncdiehl11 ncdiehl11

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@ncdiehl11 @koji @jerader @ncdiehl