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

* fix: loading logic

* fix: hide stateful selects to preserve state

* chore: format

* chore: update snapshots
---
 .../ElementSchemes/DataElementIdScheme.js      |  2 +-
 .../ElementSchemes/OrgUnitIdScheme.js          |  2 +-
 src/components/MoreOptions/MoreOptions.js      | 18 +++++++++++++++---
 .../MoreOptions/MoreOptions.module.css         |  4 ++++
 .../__snapshots__/MoreOptions.test.js.snap     |  6 +-----
 .../Page/__snapshots__/Page.test.js.snap       |  6 +-----
 6 files changed, 23 insertions(+), 15 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 9b6e3bbfe..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)
     }
 
@@ -38,9 +47,12 @@ const MoreOptions = ({
                 </div>
                 <h2 className={styles.label}>{label}</h2>
             </header>
-            <Divider />
-            <div className={styles.children} data-test={`${dataTest}-children`}>
-                {!hidden && children}
+            {!hidden && <Divider />}
+            <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 0ef469731..f7d9caaed 100644
--- a/src/components/MoreOptions/MoreOptions.module.css
+++ b/src/components/MoreOptions/MoreOptions.module.css
@@ -21,6 +21,10 @@
     margin-top: var(--spacers-dp4);
 }
 
+.hidden {
+    display: none;
+}
+
 .header svg {
     margin-left: -3px;
     margin-right: 5px;
diff --git a/src/components/MoreOptions/__snapshots__/MoreOptions.test.js.snap b/src/components/MoreOptions/__snapshots__/MoreOptions.test.js.snap
index e2a8a1be3..3b361cfa0 100644
--- a/src/components/MoreOptions/__snapshots__/MoreOptions.test.js.snap
+++ b/src/components/MoreOptions/__snapshots__/MoreOptions.test.js.snap
@@ -32,11 +32,7 @@ exports[`matches snapshot when not toggled 1`] = `
       </h2>
     </header>
     <div
-      class="jsx-1555285825 jsx-876534464 "
-      data-test="dhis2-uicore-divider"
-    />
-    <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 766987a9d..ee54ecea9 100644
--- a/src/components/Page/__snapshots__/Page.test.js.snap
+++ b/src/components/Page/__snapshots__/Page.test.js.snap
@@ -563,11 +563,7 @@ exports[`matches snapshot when not loading and a mini summary task 1`] = `
             </h2>
           </header>
           <div
-            class="jsx-1555285825 jsx-876534464 "
-            data-test="dhis2-uicore-divider"
-          />
-          <div
-            class="children"
+            class="children hidden"
             data-test="mini-job-summary-container-MoreOptions-children"
           />
         </section>