WIP [DUOS-1958][risk=no] "NIH and AnVIL use" sub-component for data submission form #1771

shaemarks · 2022-08-23T19:43:39Z

Addresses DUOS-1958

Have you read Terra's Contributing Guide lately? If not, do that first.

Label PR with a Jira ticket number and include a link to the ticket
Label PR with a security risk modifier [no, low, medium, high]
PR describes scope of changes
Get a minimum of one thumbs worth of review, preferably two if enough team members are available
Get PO sign-off for all non-trivial UI or workflow changes
Verify all tests go green
Test this change deployed correctly and works on dev environment after deployment

…to DUOS-1958 � Conflicts: � src/pages/DataSubmissionForm.js

…to DUOS-1958

connorlbark · 2022-08-23T20:18:51Z

src/components/forms/forms.js

@@ -232,6 +255,35 @@ const formInputSelect = (config) => {
  ]);
 };

+const formInputRadioButtons = (config) => {


I also added this in mine! I haven't added tests yet but mine also has the ability to have 'Other' with text fields and such. Not sure how exactly we should merge them. My code is here #1747 if you want to check it out. It's a bit more complicated but it would work here too.

these merges are going to be rough! I added some radio buttons too, but yall wrote way better code XD

Yeah, I think I'll copy over the radio buttons from #1747 to my branch to make merging easier since Connor's version can work for both of us

lu-c · 2022-08-24T06:01:45Z

src/components/forms/forms.js

+        }
+      })
+    ),
+    error && div({ className: `error-message fadein`}, [


Since this repeats multiple times through the code, can we pull this out into a separate function? Sorry, that was my bad!

lu-c · 2022-08-24T06:02:52Z

src/components/forms/forms.js

@@ -30,6 +33,10 @@ export const FormValidators = {
  REQUIRED: {
    isValid: (value) => value !== undefined && value !== null && value !== '',
    msg: 'Please enter a value'
+  },
+  EMAIL: {


This is a great addition!

lu-c · 2022-08-24T06:05:54Z

src/components/forms/forms.js

+const clearDependentFormFields = (config) => {
+  const {onChange, dependentFormFields = []} = config;
+  /*
+   * Clears fields for questions that are rendered conditionally, setting them to their default value


I think of "clear" as to set it to blank, like '', while "reset" would return to the default value - There's a real value to having both of these functions! What would you think of adding a param to allow you to completely clear to type.defaultValue?

Actually... I don't know, the more I'm thinking about it, the more I'm wondering if this is necessary! Should handling behavior of dependent form components be a job of the global forms functions? The parent formField could handle clearing child form values through the onChange option that's already available.

lu-c · 2022-08-24T06:09:10Z

src/pages/DataSubmissionForm.js

@@ -41,7 +42,8 @@ export const DataSubmissionForm = () => {
    ]),

    form({}, [
-      h(DataSubmissionStudyInformation, { onChange })
+      h(DataSubmissionStudyInformation, { onChange }),
+      h(DataSubmissionNihAnvilUse, { onChange, formData })


The study information component doesn't take in any existing formData - should it? (is the purpose to set values for a saved submission form?)

Thanks for the feedback!

You can ignore the formData prop for now!
I was trying to work out some bugs with my ticket and that was left over from my experimentation. I think I found a workaround, but let me know if you have ideas on a better approach (I'll comment closer to the actual code to provide context)

…update formData for some reason)

shaemarks · 2022-08-24T14:37:59Z

src/pages/DataSubmissionForm.js

@@ -14,7 +15,7 @@ export const DataSubmissionForm = () => {
  const onChange = ({ key, value, isValid }) => {
    /* eslint-disable no-console */
    console.log('StudyInfo OnChange:', key, value, isValid);
-    set(key, value, formData);
+    set(formData, key, value);


Not sure why, but I couldn't get the lodash/fp version of set to actually update formData (but the regular lodash function seems to be working fine). I tested by adding console.log(formData) after the call to set, and with the lodash/fp version it always printed {}

shaemarks · 2022-08-24T15:06:17Z

src/components/data_submission/DataSubmissionNihAnvilUse.js

+      ],
+      validators: [FormValidators.REQUIRED],
+      onChange,
+      setFormValueParent: setNihAnvilUse


My ticket requires different questions to be rendered based on the option selected for the nihAnvilUse, but the formValue state variable is contained within the FormField itself. To address this, I'm also tracking the value of nihAnvilUse in the DataSubmissionNihAnvilUse component.

Past Attempts:
I tried experimenting getting the value from formData.nihAnvilUse, but the changes didn't register in my component since the same formData object is being used. I also tried having formData be a state variable, and in onChange, I would deepClone the current contents of formData, set the given key value pair, and call setFormData with the new object. However, this caused the hook in DataSubmissionStudyInformation to be called way too often.

Current Approach:
Currently, I am passing setNihAnvilUse as an optional setFormValueParent prop to FormField, which is called with the new form field value in onFormInputChange. This ensures that when the formValue is changed in the FormField, it is also changed in this component

shaemarks · 2022-08-24T15:16:38Z

src/components/data_submission/DataSubmissionNihAnvilUse.js

+        {label: NO, value: NO},
+      ],
+      dependentFormFields: [
+        {id: 'submittingToAnvil', type: FormFieldTypes.RADIO, shouldClear: ({value}) => value === I_DID},


I originally had all of the dependent form fields be cleared when nihAnvilUse is changed, but there are two values of nihAnvilUse where the submittingToAnvil question appears. So, if I switched between nihAnvilUse === 'I will' and 'nihAnvilUse' === 'No', submittingToAnvil in formData is cleared but the selected radio buttons weren't. One possible solution is allowing the dependentFormFields to have a shouldClear condition (if not provided, assume clear on any change). Here, submittingToAnvil is only cleared in the formData when the question is no longer rendered, so that the value in formData and the selected radio button are consistent with each other

…t component

connorlbark · 2022-09-19T16:05:32Z

closing this - redone in #1800

Shae Marks added 10 commits August 18, 2022 11:46

start

ed2066c

Merge branch 'develop' of https://github.com/DataBiosphere/duos-ui in…

5a55d33

…to DUOS-1958 � Conflicts: � src/pages/DataSubmissionForm.js

Allow radio button checkedStyle to be overridden

edfc63d

Add form fields to NihAnvilUse section

63c4da9

Add radio button input and clearDependentFormFields to forms.js

2f25db6

Add nihAnvilUse section to DataSubmissionForm

3d2537a

Fix quotes

c0cbb3d

Add cypress tests

40118aa

Merge branch 'develop' of https://github.com/DataBiosphere/duos-ui in…

c2877c9

…to DUOS-1958

Minor fixes

99e5b9c

connorlbark reviewed Aug 23, 2022

View reviewed changes

lu-c reviewed Aug 24, 2022

View reviewed changes

Shae Marks added 3 commits August 24, 2022 10:19

Bug fixes and put error message as separate function

5aa2277

track nihAnvilUse in parent component for conditional rendering

d81dfc8

Use lodash set instead of lodash/fp set (couldn't get it to actually …

560046d

…update formData for some reason)

shaemarks commented Aug 24, 2022

View reviewed changes

Shae Marks added 2 commits August 25, 2022 14:22

Move clearDependentFormFields out of forms.js and keep close to paren…

b63c097

…t component

Change shouldClear from a function to a boolean

0a582f2

connorlbark closed this Sep 19, 2022

rushtong deleted the DUOS-1958 branch December 9, 2022 19:24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WIP [DUOS-1958][risk=no] "NIH and AnVIL use" sub-component for data submission form #1771

WIP [DUOS-1958][risk=no] "NIH and AnVIL use" sub-component for data submission form #1771

shaemarks commented Aug 23, 2022

connorlbark Aug 23, 2022

lu-c Aug 24, 2022

shaemarks Aug 24, 2022

lu-c Aug 24, 2022

lu-c Aug 24, 2022

lu-c Aug 24, 2022

lu-c Aug 25, 2022 •

edited

Loading

lu-c Aug 24, 2022

shaemarks Aug 24, 2022

shaemarks Aug 24, 2022

shaemarks Aug 24, 2022

shaemarks Aug 24, 2022

connorlbark commented Sep 19, 2022

WIP [DUOS-1958][risk=no] "NIH and AnVIL use" sub-component for data submission form #1771

WIP [DUOS-1958][risk=no] "NIH and AnVIL use" sub-component for data submission form #1771

Conversation

shaemarks commented Aug 23, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lu-c Aug 25, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

connorlbark commented Sep 19, 2022

lu-c Aug 25, 2022 •

edited

Loading