Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

1.32.0 + 1.32.1 inconsistently changes custom component iframe heights #8285

Closed
4 tasks done
naterush opened this issue Mar 12, 2024 · 2 comments · Fixed by #8290
Closed
4 tasks done

1.32.0 + 1.32.1 inconsistently changes custom component iframe heights #8285

naterush opened this issue Mar 12, 2024 · 2 comments · Fixed by #8290
Labels
feature:custom-components status:needs-triage Has not been triaged by the Streamlit team type:bug Something isn't working

Comments

@naterush
Copy link

naterush commented Mar 12, 2024

Checklist

  • I have searched the existing issues for similar issues.
  • I added a very descriptive title to this issue.
  • I have provided sufficient information below to help reproduce this issue.

Summary

After upgrading to Streamlit>1.32.0, our custom component sometimes default to an iframe height of 150 pixels and never update. This happens most often on Safari. Another custom component that exists for message passing and is otherwise empty also defaults to 150 pixels now, which visually breaks apps as well.

Reproducible Code Example

First, pip install mitosheet streamlit==1.31.0.

Then, run the following app:

import streamlit as st
from mitosheet.streamlit.v1 import spreadsheet

st.set_page_config(layout="wide")

st.markdown(
"""
# Mito Sheet Example

The Mito spreadsheet generates Python code from your data manipulations in a spreadsheet interface. 

To test it out, upload a CSV file and start manipulating the data. The Python code will be generated in real-time.
""")

uploaded_file = st.file_uploader("Choose a CSV file", type="csv")

if uploaded_file is not None:
    with open(uploaded_file.name, "wb") as f:
        f.write(uploaded_file.getvalue())

    # Display the spreadsheet
    dfs, code = spreadsheet(uploaded_file.name)

    # Display the code
    st.code(code)

Then, upgrade to pip install streamlit==1.32.0, and run the app again.

Steps To Reproduce

Just look at the app between these two versions of Streamlit, and you will immediately notice visual differences.

Expected Behavior

This is what you see:

Screenshot on streamlit==1.31.0

Screenshot 2024-03-12 at 11 38 20 AM

Screenshot on streamlit==1.32.0:

Normally:
Screenshot 2024-03-12 at 11 43 43 AM

Of if you're unlucky:
Screenshot 2024-03-12 at 11 36 26 AM

Current Behavior

The differences

There are two major differences between these screen shots:

  1. We render a totally empty custom component before the Mito spreadsheet -- in 31 this takes up very little space, but in 32 this takes up 150 pixels in height. This is constantly different, and is a breaking visual change that really makes apps with Mito look worse!
  2. The Mito component (in the unlucky case) defaults to 150 pixels and never updates to the proper height, making the Mito spreadsheet component non-functional.

Note that I have only been able to trigger the unlucky case on safari. It's pretty inconsistent, so it may appear on other browsers as well (I suspect it's just a performance related race condition), but I was not able to replicate it after a bit of testing.

My guess is that it's related to this PR: #8179. I'm no expert, but this made it into the 1.32.0 release, and it deals with default component height (including callbacks that update the iframe height).

Our issue is here: mito-ds/mito#1263

Is this a regression?

  • Yes, this used to work in a previous version.

Debug info

  • Streamlit version: 1.31
  • Python version: Python 3.9.9
  • Operating System: Mac Sonoma 14.4
  • Browser: all browsers have the 150 default for the empty

Additional Information

N/A

@naterush naterush added status:needs-triage Has not been triaged by the Streamlit team type:bug Something isn't working labels Mar 12, 2024
Copy link

If this issue affects you, please react with a 👍 (thumbs up emoji) to the initial post.

Your feedback helps us prioritize which bugs to investigate and address first.

Visits

@naterush naterush changed the title 1.32.0 inconsistently changes custom component iframe heights 1.32.0 + 1.32.1 inconsistently changes custom component iframe heights Mar 12, 2024
kmcgrady added a commit that referenced this issue Mar 14, 2024
## Describe your changes

In 1.32.0, we introduced some changes to custom components that rendered
an iframe of a certain height instead of a default of 0. This change
ensures that when the `frameHeight` is `undefined`, it will use 0.

## GitHub Issue Link (if applicable)
Closes #8285

## Testing Plan

- Added a test to make sure it's set (and the Skeleton appears and
disappears)

---

**Contribution License Agreement**

By submitting this pull request you agree that all contributions to this
project are made under the Apache 2.0 license.
@kmcgrady
Copy link
Collaborator

Hey @naterush ! Thanks for getting to us. We have merged a fix to this, and it will be in the next release hopefully in the next day or so.

Just want to let you know that the end state should mimic the same behavior as before. The only difference you might notice is the time period before your custom component is loaded, we will display a "skeleton". The skeleton will disappear and collapse down for the message passing. You can bypass this by sending down a height parameter to inform the system the height the component intends to be, so the loading skeleton matches the size and look seamless. So for the message passer you can send a height = 0, which will never display the loading skeleton in the first place. Similarly, the sheet itself can have a larger size if you know what it is before.

Happy to answer any more questions, but I thank you for your patience in this!

kmcgrady added a commit that referenced this issue Mar 14, 2024
## Describe your changes

In 1.32.0, we introduced some changes to custom components that rendered
an iframe of a certain height instead of a default of 0. This change
ensures that when the `frameHeight` is `undefined`, it will use 0.

## GitHub Issue Link (if applicable)
Closes #8285

## Testing Plan

- Added a test to make sure it's set (and the Skeleton appears and
disappears)

---

**Contribution License Agreement**

By submitting this pull request you agree that all contributions to this
project are made under the Apache 2.0 license.
zyxue pushed a commit to zyxue/streamlit that referenced this issue Apr 16, 2024
## Describe your changes

In 1.32.0, we introduced some changes to custom components that rendered
an iframe of a certain height instead of a default of 0. This change
ensures that when the `frameHeight` is `undefined`, it will use 0.

## GitHub Issue Link (if applicable)
Closes streamlit#8285

## Testing Plan

- Added a test to make sure it's set (and the Skeleton appears and
disappears)

---

**Contribution License Agreement**

By submitting this pull request you agree that all contributions to this
project are made under the Apache 2.0 license.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature:custom-components status:needs-triage Has not been triaged by the Streamlit team type:bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants