feat(app): Enable pipette config modal and form #3202
Conversation
Render hidden config fields behind new feat flag closes #3112
Kadee80
added
feature
Codecov Report
@@ Coverage Diff @@
## edge #3202 +/- ##
==========================================
- Coverage 53.17% 53.14% -0.03%
==========================================
Files 691 691
Lines 20403 20413 +10
==========================================
Hits 10849 10849
- Misses 9554 9564 +10
Continue to review full report at Codecov.
|
| @@ -64,6 +67,14 @@ export default class ConfigForm extends React.Component<Props> { | |||
| ]) | |||
| } | |||
|
|
|||
| getHiddenFields = () => { | |||
There was a problem hiding this comment.
Two things:
hiddenFieldsfeels a little confusing for fields that are displayed and visible- In my head I was treating these as "known fields" and "unknown fields"
- Rather than add this new method, does it make sense to move this logic to
getVisibleFields?
| @@ -84,7 +95,9 @@ export default class ConfigForm extends React.Component<Props> { | |||
|
|
|||
| validate = (values: FormValues) => { | |||
| const errors = {} | |||
| const fields = this.getVisibleFields() | |||
| const fields = this.props.showHiddenFields | |||
There was a problem hiding this comment.
This logic is duplicated here and in render and should probably be moved to getVisibleFields
There was a problem hiding this comment.
This line change should be reverted with the update to getVisibleFields
There was a problem hiding this comment.
Small weirdness: any whitespace ' ' gets cast to 0. When it goes out to the PATCH, it's 0 so it shouldn't be able to cause anything weird to happen. Also this is only possible to do in plunger fields since the others have minimum greater than zero
❌ With make -C app dev OT_APP_DEV_INTERNAL__ALL_PIPETTE_CONFIG=1, tipLength reports a min and max but the form doesn't validate the range (the API does though). That seems wrong b/c defaultAspirateFlowRate and defaultDispenseFlowRate look the same coming from GET /settings/pipettes/{pipetteId}, but the front end is validating them correctly. I'm not sure why...
| } else if (min && max && (parsed < min || value > max)) { | ||
| } else if ( | ||
| typeof min === 'number' && | ||
| max && |
There was a problem hiding this comment.
this should probably get the same typeof ... 'number' treatment, it's possible a max could be zero
| @@ -84,7 +95,9 @@ export default class ConfigForm extends React.Component<Props> { | |||
|
|
|||
| validate = (values: FormValues) => { | |||
| const errors = {} | |||
| const fields = this.getVisibleFields() | |||
| const fields = this.props.showHiddenFields | |||
There was a problem hiding this comment.
This line change should be reverted with the update to getVisibleFields
overview
This PR removes feature flag and enables pipette config for up to date robots (robots which return a response from /settings/pipettes)
This PR also renders validates and submits hidden fields when new feature flag
--internalDev.allPipetteConfigis true.closes #3112
changelog
review requests
note: I gave a build to support so I will hold off on merging this until they give me feedback on functionality
make -C app devmake -C app dev OT_APP_DEV_INTERNAL__ALL_PIPETTE_CONFIG=1