-
Notifications
You must be signed in to change notification settings - Fork 180
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(components, protocol-designer, step-generation): full absorbance reader step form UI #17306
Conversation
...src/pages/Designer/ProtocolSteps/StepForm/StepTools/AbsorbanceReaderTools/Initialization.tsx
Show resolved
Hide resolved
...src/pages/Designer/ProtocolSteps/StepForm/StepTools/AbsorbanceReaderTools/Initialization.tsx
Show resolved
Hide resolved
...src/pages/Designer/ProtocolSteps/StepForm/StepTools/AbsorbanceReaderTools/Initialization.tsx
Outdated
Show resolved
Hide resolved
...src/pages/Designer/ProtocolSteps/StepForm/StepTools/AbsorbanceReaderTools/Initialization.tsx
Outdated
Show resolved
Hide resolved
...src/pages/Designer/ProtocolSteps/StepForm/StepTools/AbsorbanceReaderTools/Initialization.tsx
Show resolved
Hide resolved
...src/pages/Designer/ProtocolSteps/StepForm/StepTools/AbsorbanceReaderTools/Initialization.tsx
Outdated
Show resolved
Hide resolved
...s/Designer/ProtocolSteps/StepForm/StepTools/AbsorbanceReaderTools/InitializationSettings.tsx
Outdated
Show resolved
Hide resolved
...er/src/pages/Designer/ProtocolSteps/StepForm/StepTools/AbsorbanceReaderTools/LidControls.tsx
Outdated
Show resolved
Hide resolved
...r/src/pages/Designer/ProtocolSteps/StepForm/StepTools/AbsorbanceReaderTools/ReadSettings.tsx
Show resolved
Hide resolved
...designer/src/pages/Designer/ProtocolSteps/StepForm/StepTools/AbsorbanceReaderTools/index.tsx
Outdated
Show resolved
Hide resolved
moduleId: null, | ||
referenceWavelength: null, | ||
wavelengths: null, | ||
referenceWavelengthActive: false, | ||
wavelengths: [''], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how come wavelengths can be defaulted to an empty array []
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The reason here is because we always want to show at least one wavelength option. If we don't do this, we need to add a useEffect and some other ugliness upon mounting. I think this is the cleanest solution, as this property is ignored unless the form type is initialize
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i see, i guess this doesn't actually affect anything to have it an empty string here.
protocol-designer/src/steplist/formLevel/stepFormToArgs/absorbanceReaderFormToArgs.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
woohoo! this is exciting progress, great to see this progress with the form!
… reader step form UI (#17306) This PR completes the UI for absorbance reader step form, specifically creating the initialization editor component. It also adds form errors for all implicated fields throughout the form. Note that because all wavelengths are actually produced by the same form field, logic for handling each individual wavelength error is handled within the component rather than our existing infrastructure for form/fieldLevel errors. Closes AUTH-1267, Closes AUTH-1315
Overview
This PR completes the UI for absorbance reader step form, specifically creating the initialization editor component. It also adds form errors for all implicated fields throughout the form.
Note that because all wavelengths are actually produced by the same form field, logic for handling each individual wavelength error is handled within the component rather than our existing infrastructure for form/fieldLevel errors.
Closes AUTH-1267, Closes AUTH-1315
Test Plan and Hands on Testing
Changelog
Review requests
see test plan
Risk assessment
low. should not implicate any other parts of PD not related to the absorbance reader step form