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(protocol-designer): finish form-level errors for dynamic fields #5818

Merged
merged 1 commit into from
Jun 5, 2020

Conversation

IanLondon
Copy link
Contributor

overview

Closes #5814

changelog

  • Show form-level errors for steps that are inside a cycle (not just top-level steps)
  • Disable save when form-level errors are present

review requests

  • Show error when both minutes and seconds are blank or zero
  • Don't show that error when either minutes or seconds hasn't been blurred
  • Error should prevent save (whether it's visible or not)
  • This should work both in top-level profile steps, and steps in cycles

NOTE: since the errors are not associated with a particular row, when any field of a given name is pristine, all will be treated as pristine. So adding a new step will hide any previously visible errors.

(There's just one error, if you don't have a > 0 mins + seconds)

risk assessment

low, PD-only and under a FF

* Show form-level errors for steps that are inside a cycle (not just top-level steps)
* Disable save when form-level errors are present
@IanLondon IanLondon added protocol designer Affects the `protocol-designer` project 🕷️ SPDDRS labels Jun 4, 2020
@IanLondon IanLondon requested review from Kadee80, shlokamin and a team and removed request for a team June 4, 2020 18:26
Copy link
Contributor

@Kadee80 Kadee80 left a comment

Choose a reason for hiding this comment

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

🐺 works as expected!

@@ -432,7 +432,10 @@ const _dynamicFieldFormErrors = (
export const _hasFieldLevelErrors = (hydratedForm: FormData): boolean => {
for (const fieldName in hydratedForm) {
const value = hydratedForm[fieldName]
if (fieldName === 'profileItemsById') {
if (
hydratedForm.stepType === 'thermocycler' &&
Copy link
Contributor Author

Choose a reason for hiding this comment

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

after re-reading this a few times I prefer it to be explicit: this is only for thermocycler forms, no other forms

Copy link
Member

Choose a reason for hiding this comment

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

yeah i like that

Copy link
Member

@shlokamin shlokamin left a comment

Choose a reason for hiding this comment

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

This looks solid! I notice that for error checking in general we don't seem to have much unit test coverage. I think it would be worth thinking about that all together, because error logic is pretty important.

@IanLondon IanLondon merged commit 9ca9911 into edge Jun 5, 2020
@IanLondon IanLondon deleted the pd_finish-formlevel-validation branch June 5, 2020 16:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
protocol designer Affects the `protocol-designer` project
Projects
None yet
Development

Successfully merging this pull request may close these issues.

PD: Form-level dynamic field validation (TC Profile)
3 participants