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

_remove_extra_spacing() hides other components #67

Open
benlindsay opened this issue Apr 2, 2024 · 11 comments
Open

_remove_extra_spacing() hides other components #67

benlindsay opened this issue Apr 2, 2024 · 11 comments

Comments

@benlindsay
Copy link

benlindsay commented Apr 2, 2024

First off thanks for maintaining this package! Very helpful stuff. Just noticed one thing in the latest release. The _remove_extra_spacing() method of CookieManager has too broad of reach. Other packages, like streamlit-oauth, have 0-height components, so that style definition hides that too. As an example, with streamlit==1.32.2, extra-streamlit-components==0.1.71, and streamlit-oauth==0.1.8 installed:

# visible_button.py
import streamlit as st
from streamlit_oauth import OAuth2Component

oauth2 = OAuth2Component("abc", "123", "https://example.com/auth", "https://example.com/token")
oauth2.authorize_button("Authorize", "https://example.com/redirect", "token")

shows the authorize button, but

# invisible_button.py

import streamlit as st
from streamlit_oauth import OAuth2Component
import extra_streamlit_components as stx

cookie_manager = stx.CookieManager()
cookies = cookie_manager.get_all()
oauth2 = OAuth2Component("abc", "123", "https://example.com/auth", "https://example.com/token")
oauth2.authorize_button("Authorize", "https://example.com/redirect", "token")

shows nothing.

What I've been using in my app is something like this:

import streamlit as st

st.markdown(
    """
    <style>
        .element-container:has(
            iframe[title="extra_streamlit_components.CookieManager.cookie_manager"]
        ) {
            display: none
        }
    </style>
    """,
    unsafe_allow_html=True,
)

I can't find where I originally saw that or something like it to attribute the idea to the right person, but could we switch to something narrower like this to avoid hiding other components?

@juancotrino
Copy link

Hi! This happens with the streamlit-option-menu package as well. @benlindsay where would you place that workaround piece of code? In the _remove_extra_spacing() method itself?

@benlindsay
Copy link
Author

Yeah. Without replacing the CSS in _remove_extra_spacing(), I don't know of any way to add additional CSS to undo it. This makes the latest version unusable for me. I'm currently keeping it pinned at <0.1.71

@map0logo
Copy link

map0logo commented Aug 4, 2024

Hi, I have the same problem. When I use stx.CookieManager() in the same page as streamlit-agraph, the graph disappear, I've tried to patch the html, but the agraph canvas take initial height="0", so the patch didn't work.

My main question is, why is _remove_extra_spacing() needed? What is it supposed to achieve, is it safe to change it or remove it?

@benlindsay
Copy link
Author

Without that function or something like it, every time the Cookie Manager widget is called, a blank element that's 10 or 15 pixels tall (can't remember which) shows up. This can lead to different amounts of extra spacing in your app depending on how many times the Cookie Manager gets called in a particular run, which can be an annoying user experience.

@informatica92
Copy link

This happens also with Streamlit Searchbox.

Anyway, IMHO, I think that 10, 15, or even 100 empty pixels is much better then hiding other components.

@map0logo
Copy link

map0logo commented Oct 5, 2024

However, if you have isolated pages that need to read some cookies now you can use context cookies https://docs.streamlit.io/develop/api-reference/utilities/st.context#contextcookies

@benlindsay
Copy link
Author

benlindsay commented Oct 6, 2024

Anyway, IMHO, I think that 10, 15, or even 100 empty pixels is much better then hiding other components.

I 100% agree. I think the proposed change I made in the original comment on this issue would be pretty targeted. I'd be very happy with just having the whole method removed though, since that would make the latest commit usable again. Currently it is broken.

@Mohamed-512 any chance you'll be able to address this at some point?

@informatica92
Copy link

However, if you have isolated pages that need to read some cookies now you can use context cookies https://docs.streamlit.io/develop/api-reference/utilities/st.context#contextcookies

I made the same assumption but I noticed that the built-in method you mentioned does NOT work in Community Cloud deployments

As you can see from this linked discussion in the Streamlit Cloud Forum

@benlindsay
Copy link
Author

@informatica92 Oof that's frustrating, good to know. Should be a useful strategy in k8s where my app is deployed though

@emigre459
Copy link

+1 for a fix to this, caused me all sorts of confusion when using streamlit-authenticator until I realized the issue at hand by reading this thread.

In general, unfortunately I've found the CookieManager to be very difficult to work with (if you accidentally have two instances for example, they can conflict and cause misinterpretation of the current cookies in the browser). Leveraging native st.context.cookies functionality has helped a lot here, but still much to be done.

@heeki
Copy link

heeki commented Jan 17, 2025

I had the same issue with streamlit-calendar. I used an alternate cookie manager and was able to get the calendar to render. I posted an issue there and figured I'd cross-reference it here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants