fix(components, protocol-designer): fix logic for disabling advanced transfer settings #16996
+66
−10
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…transfer settings Update logic for disabling mix/blowout settings depending on transfer path. If path is consolidate, aspirate mix field should be disabled. If path is distribute, dispense mix field should be disabled, and blowout field should be disabled if disposal volume field is enabled. Side effect of enabling disposal volume field is to disable and collapse blowout field. These changes implicate that Check and ListItem components accommodate disabled state.
koji
reviewed
Dec 2, 2024
koji
reviewed
Dec 2, 2024
koji
approved these changes
Dec 2, 2024
jerader
reviewed
Dec 2, 2024
Comment on lines
+355
to
+364
| tooltipText={ | ||
| tab === 'dispense' | ||
| ? dispenseMixDisabledTooltipText | ||
| : propsForFields.aspirate_mix_checkbox.tooltipContent | ||
| } | ||
| disabled={ | ||
| tab === 'dispense' | ||
| ? isDestinationTrash || formData.path === 'multiDispense' | ||
| : formData.path === 'multiAspirate' | ||
| } |
There was a problem hiding this comment.
nit: this is ok for now but would be nice to refactor this in the future to try to get the logic upstream with propsForFields.aspirate_mix_checkbox.tooltipContent and propsForFields.aspirate_mix_checkbox.disabled to keep things organized.
There was a problem hiding this comment.
I agree, especially if this logic is expected to through for several other fields. I'll ticket to address in followup
jerader
reviewed
Dec 2, 2024
Comment on lines
+451
to
+454
| disabled={ | ||
| formData.path === 'multiDispense' && | ||
| formData.disposalVolume_checkbox | ||
| } |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Overview
Update logic for disabling mix/blowout settings depending on transfer path.
Side effect of enabling disposal volume field is to disable and collapse blowout field. These changes implicate that Check and ListItem components accommodate disabled state.
Note that I don't love this fix— it breaks pattern with how we handle tooltips, but it looks like we haven't had to deal with multiple disabled tooltips on the same form field in the past. I am open to ideas on how to migrate this to a more sustainable pattern moving forward.
Test Plan and Hands on Testing
consolidate
distribute
Changelog
Review requests
see test plan
Risk assessment
low