From 6d74c8f00a46436312f294445cdefaa4f0938b31 Mon Sep 17 00:00:00 2001
From: Vincent Donato <vincent@streamlit.io>
Date: Thu, 20 Jan 2022 16:36:04 -0800
Subject: [PATCH] Don't retain state across distinct-but-similar expanders
 (#4290)

* Don't retain state across distinct-but-similar expanders

* Add explanatory comment
---
 e2e/scripts/expander_state.py                 | 12 ++++++
 e2e/specs/expander_state.spec.js              | 40 +++++++++++++++++++
 .../hocs/withExpandable/withExpandable.tsx    | 16 ++++++--
 3 files changed, 64 insertions(+), 4 deletions(-)
 create mode 100644 e2e/scripts/expander_state.py
 create mode 100644 e2e/specs/expander_state.spec.js

diff --git a/e2e/scripts/expander_state.py b/e2e/scripts/expander_state.py
new file mode 100644
index 000000000000..217b98fc87b3
--- /dev/null
+++ b/e2e/scripts/expander_state.py
@@ -0,0 +1,12 @@
+import streamlit as st
+
+b0 = st.button("b0")
+b1 = st.button("b1")
+
+if b0:
+    with st.expander("b0_expander", expanded=False):
+        st.write("b0_write")
+
+if b1:
+    with st.expander("b1_expander", expanded=False):
+        st.write("b1_write")
diff --git a/e2e/specs/expander_state.spec.js b/e2e/specs/expander_state.spec.js
new file mode 100644
index 000000000000..6d15a8ec4a72
--- /dev/null
+++ b/e2e/specs/expander_state.spec.js
@@ -0,0 +1,40 @@
+/**
+ * @license
+ * Copyright 2018-2022 Streamlit Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+describe("expandable state", () => {
+  before(() => {
+    cy.loadApp("http://localhost:3000/");
+  });
+
+  it("does not retain expander state for a distinct expander", () => {
+    cy.getIndexed(".stButton button", 0).click();
+    cy.get("[data-testid='stExpander']").click();
+
+    cy.get("[data-testid='stExpander']").should("contain.text", "b0_write");
+
+    cy.getIndexed(".stButton button", 1).click();
+
+    cy.get("[data-testid='stExpander']").should(
+      "not.contain.text",
+      "b0_write"
+    );
+    cy.get("[data-testid='stExpander']").should(
+      "not.contain.text",
+      "b1_write"
+    );
+  });
+});
diff --git a/frontend/src/hocs/withExpandable/withExpandable.tsx b/frontend/src/hocs/withExpandable/withExpandable.tsx
index 510df9fb306a..ab91b08b8a2d 100644
--- a/frontend/src/hocs/withExpandable/withExpandable.tsx
+++ b/frontend/src/hocs/withExpandable/withExpandable.tsx
@@ -48,12 +48,20 @@ function withExpandable(
       ...componentProps
     } = props
 
-    const [expanded, toggleExpanded] = useState<boolean>(initialExpanded)
+    const [expanded, setExpanded] = useState<boolean>(initialExpanded)
     useEffect(() => {
-      toggleExpanded(initialExpanded)
-    }, [initialExpanded])
+      setExpanded(initialExpanded)
+      // Having `label` in the dependency array here is necessary because
+      // sometimes two distinct expanders look so similar that even the react
+      // diffing algorithm decides that they're the same element with updated
+      // props (this happens when something in the app removes one expander and
+      // replaces it with another in the same position).
+      //
+      // By adding `label` as a dependency, we ensure that we reset the
+      // expander's `expanded` state in this edge case.
+    }, [label, initialExpanded])
 
-    const toggle = (): void => toggleExpanded(!expanded)
+    const toggle = (): void => setExpanded(!expanded)
     const { colors, radii, spacing, fontSizes } = useTheme<Theme>()
 
     return (