Repository: streamlit/streamlit
Pull request: Fix issue with hidden balloons (approach 2)
## 📚 Context

The [`Block.tsx` refactoring](https://github.com/streamlit/streamlit/pull/4053) applies a `display: none` to the element containers of `empty` and `balloons` elements. The reason is to prevent the `gap` of the parent flexbox. However, the `balloon` element is never shown on the frontend due to `display: none`. This PR applies negative bottom margin to balloons in the same size of the gap to prevent the gap. With this workaround, we can render the `balloons` element.

This PR also removes the `isHidden` flag, since it is not anymore needed as abstraction since `empty` and `balloons` are now treated differently.

- What kind of change does this PR introduce?

  - [x] Bugfix
  - [ ] Feature
  - [ ] Refactoring
  - [ ] Other, please describe:

## 🧪 Testing Done

- [ ] Screenshots included
- [ ] Added/Updated unit tests
- [ ] Added/Updated e2e tests

---

**Contribution License Agreement**

By submitting this pull request you agree that all contributions to this project are made under the Apache 2.0 license.


Problem statement:
st.balloons() is not working in streamlit 1.3.0
### Summary

st.balloons() was working in 1.2.0 version, but not in 1.3.0.
(The st.success text is executing all right on both versions)

Code snippet:

```
with st.spinner("Analyzing..."):
        pass
st.success("Done!")
st.balloons()
```
### Is this a regression?

That is, did this use to work the way you expected in the past?
yes

### Debug info

- Streamlit version: 1.3.0
- Python version: 3.9

Hints from issue discussion:
Thanks for reporting this @RohanKaran!

This is known, and we have two proposed fixes out (#4199 and #4204) and are deciding between the two. We'll have a fix out ASAP.

Commit message: Pin mypy to older version

Patch to review:
diff --git a/e2e/scripts/st_balloons.py b/e2e/scripts/st_balloons.py
new file mode 100644
index 000000000000..32af104a16e8
--- /dev/null
+++ b/e2e/scripts/st_balloons.py
@@ -0,0 +1,17 @@
+# Copyright 2018-2021 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.
+
+import streamlit as st
+
+st.balloons()
diff --git a/e2e/specs/st_balloons.spec.js b/e2e/specs/st_balloons.spec.js
new file mode 100644
index 000000000000..da5ef5b0dc46
--- /dev/null
+++ b/e2e/specs/st_balloons.spec.js
@@ -0,0 +1,35 @@
+/**
+ * @license
+ * Copyright 2018-2021 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("st.balloons", () => {
+  before(() => {
+    cy.visit("http://localhost:3000/");
+  });
+
+  it("uses negative bottom margin styling", () => {
+    // balloons use negative bottom margin to prevent the flexbox gap (instead of display: none like st.empty)
+    cy.get(".balloons")
+      .eq(0)
+      .parent()
+      .should("have.css", "margin-bottom");
+
+    cy.get(".balloons")
+      .eq(0)
+      .parent()
+      .should("not.have.css", "display", "none");
+  });
+});
diff --git a/e2e/specs/st_empty.spec.js b/e2e/specs/st_empty.spec.js
index 3eb787235650..03dd0f281426 100644
--- a/e2e/specs/st_empty.spec.js
+++ b/e2e/specs/st_empty.spec.js
@@ -22,4 +22,11 @@ describe("st.empty", () => {
     // Make the ribbon decoration line disappear
     cy.get("[data-testid='stDecoration']").invoke("css", "display", "none");
   });
+
+  it("uses display none styling", () => {
+    cy.get(".stHidden")
+      .eq(0)
+      .parent()
+      .should("have.css", "display", "none");
+  });
 });
diff --git a/frontend/src/components/core/Block/Block.tsx b/frontend/src/components/core/Block/Block.tsx
index ff5314134940..78a75621c62f 100644
--- a/frontend/src/components/core/Block/Block.tsx
+++ b/frontend/src/components/core/Block/Block.tsx
@@ -57,7 +57,7 @@ const BlockNodeRenderer = (props: BlockPropsWithWidth): ReactElement => {
     return <></>
   }
 
-  const enable = shouldComponentBeEnabled(false, props.reportRunState)
+  const enable = shouldComponentBeEnabled("", props.reportRunState)
   const isStale = isComponentStale(
     enable,
     node,
diff --git a/frontend/src/components/core/Block/ElementNodeRenderer.tsx b/frontend/src/components/core/Block/ElementNodeRenderer.tsx
index 749e368d59de..67d5f6d478ba 100644
--- a/frontend/src/components/core/Block/ElementNodeRenderer.tsx
+++ b/frontend/src/components/core/Block/ElementNodeRenderer.tsx
@@ -570,9 +570,8 @@ const ElementNodeRenderer = (
 ): ReactElement => {
   const { node } = props
 
-  const elementType = node.element.type
-  const isHidden = elementType === "empty" || elementType === "balloons"
-  const enable = shouldComponentBeEnabled(isHidden, props.reportRunState)
+  const elementType = node.element.type || ""
+  const enable = shouldComponentBeEnabled(elementType, props.reportRunState)
   const isStale = isComponentStale(
     enable,
     node,
@@ -595,9 +594,9 @@ const ElementNodeRenderer = (
       <StyledElementContainer
         data-stale={isStale}
         isStale={isStale}
-        isHidden={isHidden}
+        width={width}
         className={"element-container"}
-        style={{ width, display: isHidden ? "none" : undefined }}
+        elementType={elementType}
       >
         <ErrorBoundary width={width}>
           <Suspense
diff --git a/frontend/src/components/core/Block/styled-components.ts b/frontend/src/components/core/Block/styled-components.ts
index 0240742e3062..7d1ab98631c8 100644
--- a/frontend/src/components/core/Block/styled-components.ts
+++ b/frontend/src/components/core/Block/styled-components.ts
@@ -32,11 +32,13 @@ export const StyledHorizontalBlock = styled.div(({ theme }) => ({
 
 export interface StyledElementContainerProps {
   isStale: boolean
-  isHidden: boolean
+  width: number
+  elementType: string
 }
 
 export const StyledElementContainer = styled.div<StyledElementContainerProps>(
-  ({ theme, isStale, isHidden }) => ({
+  ({ theme, isStale, width, elementType }) => ({
+    width,
     // Allows to have absolutely-positioned nodes inside report elements, like
     // floating buttons.
     position: "relative",
@@ -54,6 +56,19 @@ export const StyledElementContainer = styled.div<StyledElementContainerProps>(
           transition: "opacity 1s ease-in 0.5s",
         }
       : {}),
+    ...(elementType === "empty"
+      ? {
+          // Use display: none for empty elements to avoid the flexbox gap.
+          display: "none",
+        }
+      : {}),
+    ...(elementType === "balloons"
+      ? {
+          // Apply negative bottom margin to remove the flexbox gap.
+          // display: none does not work for balloons, since it needs to be visible.
+          marginBottom: `-${theme.spacing.lg}`,
+        }
+      : {}),
   })
 )
 
diff --git a/frontend/src/components/core/Block/utils.ts b/frontend/src/components/core/Block/utils.ts
index ecd58b97599d..2df49e5d2cbd 100644
--- a/frontend/src/components/core/Block/utils.ts
+++ b/frontend/src/components/core/Block/utils.ts
@@ -22,10 +22,10 @@ import { FileUploadClient } from "src/lib/FileUploadClient"
 import { ComponentRegistry } from "src/components/widgets/CustomComponent/"
 
 export function shouldComponentBeEnabled(
-  isHidden: boolean,
+  elementType: string,
   reportRunState: ReportRunState
 ): boolean {
-  return !isHidden || reportRunState !== ReportRunState.RUNNING
+  return elementType !== "empty" || reportRunState !== ReportRunState.RUNNING
 }
 
 export function isElementStale(
diff --git a/lib/Pipfile b/lib/Pipfile
index 546a2f21326d..374abf2a1ea6 100644
--- a/lib/Pipfile
+++ b/lib/Pipfile
@@ -9,7 +9,7 @@ black = "==21.6b0"
 boto3 = "*"
 botocore = ">=1.13.44"
 hypothesis = ">=6.17.4"
-mypy = ">=0.910"
+mypy = ">=0.910, <0.930"
 mypy-protobuf = ">=1.17"
 parameterized = "*"
 pipenv = "*"


Code review comments:
diff_hunk: @@ -9,7 +9,7 @@ black = "==21.6b0"
 boto3 = "*"
 botocore = ">=1.13.44"
 hypothesis = ">=6.17.4"
-mypy = ">=0.910"
+mypy = ">=0.910, <0.930"
path: lib/Pipfile
text: @user1:
I don't think we should do this (in fact, if anything, we should pin `mypy = ">=0.930"`)

@author:
That's was just a test if my build or mypy is broken. And as it turns out, mypy `0.930` (which was released a few hours ago) breaks our build pipelines :(

@author:
See the build logs here: https://app.circleci.com/pipelines/github/streamlit/streamlit/10654/workflows/9f4fcc98-06ec-474d-b7b8-84907ef9c7ca/jobs/34844/parallel-runs/0/steps/0-123

@user2:
I think it may just be necessary to pull in the latest changes from `develop`, since I believe https://github.com/streamlit/streamlit/pull/4191 fixed this

@author:
I will try that. However, the builds went fine yesterday and those issues only appeared after mypy 0.930 got released. So, I'm assuming there are additional issues 😬

@author:
Actually, the fixes from https://github.com/streamlit/streamlit/pull/4191 are already included in this branch.

@user2:
Ah, #4191 fixed the issues that appeared with mypy 0.920

I didn't realize this was a new set of issues that appeared with the release of 0.930 😬 

@user1:
Annoying!

It looks like all these issues are in the `legacy_caching/hashing.py` file - as a stopgap, we could add `# type: ignore` to the top of that file, along with a TODO to investigate after the break. (#core-platform can be in charge of fixing this issue.)

@user3:
We got a solution with https://github.com/streamlit/streamlit/pull/4218 that fixes with the latest mypy. I've merged it in this one.