From 18ee74333c27b0628fddc36728767183122dca40 Mon Sep 17 00:00:00 2001
From: Kai Vandivier <49666798+KaiVandivier@users.noreply.github.com>
Date: Fri, 15 Dec 2023 17:12:54 +0100
Subject: [PATCH] fix: avoid breaking selects [DHIS2-16264] (#1963)

* fix: loading logic

* fix: hide stateful selects to preserve state

* chore: format

* chore: update snapshots
---
 .../ElementSchemes/DataElementIdScheme.js        |  2 +-
 src/components/ElementSchemes/OrgUnitIdScheme.js |  2 +-
 src/components/MoreOptions/MoreOptions.js        | 16 ++++++++++++++--
 .../MoreOptions/MoreOptions.module.css           |  4 ++++
 .../__snapshots__/MoreOptions.test.js.snap       |  2 +-
 .../Page/__snapshots__/Page.test.js.snap         |  2 +-
 6 files changed, 22 insertions(+), 6 deletions(-)

diff --git a/src/components/ElementSchemes/DataElementIdScheme.js b/src/components/ElementSchemes/DataElementIdScheme.js
index c2c7e2777..51b7c13f6 100644
--- a/src/components/ElementSchemes/DataElementIdScheme.js
+++ b/src/components/ElementSchemes/DataElementIdScheme.js
@@ -22,7 +22,7 @@ const DataElementIdScheme = ({
         fetchAttributes(`${baseUrl}/api/`, 'dataElementAttribute')
             .then((attributes) => setSchemes(attributes))
             .catch((error) => setError(error))
-        setLoading(false)
+            .finally(() => setLoading(false))
     }, [])
 
     const validationText =
diff --git a/src/components/ElementSchemes/OrgUnitIdScheme.js b/src/components/ElementSchemes/OrgUnitIdScheme.js
index 1d5c373a3..47511c650 100644
--- a/src/components/ElementSchemes/OrgUnitIdScheme.js
+++ b/src/components/ElementSchemes/OrgUnitIdScheme.js
@@ -17,7 +17,7 @@ const OrgUnitIdScheme = ({ name, label, orgUnitIdSchemeOptions, dataTest }) => {
         fetchAttributes(`${baseUrl}/api/`, 'organisationUnitAttribute')
             .then((attributes) => setSchemes(attributes))
             .catch((error) => setError(error))
-        setLoading(false)
+            .finally(() => setLoading(false))
     }, [])
 
     const validationText =
diff --git a/src/components/MoreOptions/MoreOptions.js b/src/components/MoreOptions/MoreOptions.js
index 837d0aaab..d1ac59935 100644
--- a/src/components/MoreOptions/MoreOptions.js
+++ b/src/components/MoreOptions/MoreOptions.js
@@ -5,6 +5,11 @@ import PropTypes from 'prop-types'
 import React, { useState } from 'react'
 import styles from './MoreOptions.module.css'
 
+/**
+ * If not initially visible, doesn't render children. After opening the first
+ * time, however, children are instead hidden by CSS on toggle to preserve
+ * state
+ */
 const MoreOptions = ({
     children,
     initiallyVisible,
@@ -13,8 +18,12 @@ const MoreOptions = ({
     dataTest = 'interaction-more-options',
 }) => {
     const [hidden, setHidden] = useState(!initiallyVisible)
+    const [hasOpened, setHasOpened] = useState(initiallyVisible)
 
     const onToggle = () => {
+        if (!hasOpened) {
+            setHasOpened(true)
+        }
         setHidden(!hidden)
     }
 
@@ -39,8 +48,11 @@ const MoreOptions = ({
                 <h2 className={styles.label}>{label}</h2>
             </header>
             {!hidden && <Divider />}
-            <div className={styles.children} data-test={`${dataTest}-children`}>
-                {!hidden && children}
+            <div
+                className={cx(styles.children, { [styles.hidden]: hidden })}
+                data-test={`${dataTest}-children`}
+            >
+                {hasOpened && children}
             </div>
         </section>
     )
diff --git a/src/components/MoreOptions/MoreOptions.module.css b/src/components/MoreOptions/MoreOptions.module.css
index e60c1e6b5..e584c4f95 100644
--- a/src/components/MoreOptions/MoreOptions.module.css
+++ b/src/components/MoreOptions/MoreOptions.module.css
@@ -29,6 +29,10 @@
     margin-top: var(--spacers-dp4);
 }
 
+.hidden {
+    display: none;
+}
+
 .chevronHidden svg {
     transform: rotate(90deg);
 }
diff --git a/src/components/MoreOptions/__snapshots__/MoreOptions.test.js.snap b/src/components/MoreOptions/__snapshots__/MoreOptions.test.js.snap
index 924a75aa4..3b361cfa0 100644
--- a/src/components/MoreOptions/__snapshots__/MoreOptions.test.js.snap
+++ b/src/components/MoreOptions/__snapshots__/MoreOptions.test.js.snap
@@ -32,7 +32,7 @@ exports[`matches snapshot when not toggled 1`] = `
       </h2>
     </header>
     <div
-      class="children"
+      class="children hidden"
       data-test="more-options-children"
     />
   </section>
diff --git a/src/components/Page/__snapshots__/Page.test.js.snap b/src/components/Page/__snapshots__/Page.test.js.snap
index 085b6d726..ee54ecea9 100644
--- a/src/components/Page/__snapshots__/Page.test.js.snap
+++ b/src/components/Page/__snapshots__/Page.test.js.snap
@@ -563,7 +563,7 @@ exports[`matches snapshot when not loading and a mini summary task 1`] = `
             </h2>
           </header>
           <div
-            class="children"
+            class="children hidden"
             data-test="mini-job-summary-container-MoreOptions-children"
           />
         </section>