Skip to content

Conversation

@lukasmasuch
Copy link
Collaborator

@lukasmasuch lukasmasuch commented Aug 26, 2025

Describe your changes

Fixes an issue with the logo pushing out the collapse button. This was already working fine when a link was set -> the reason is that the additional wrapping leads to a more correct calculation of the actual available width. I think this is also solvable differently with some CSS stuff, but I think its ok to just wrap the logo in a div when its not already wrapped into a a element.

GitHub Issue Link (if applicable)

Testing Plan

  • Updated the e2e tests to ensure that the collapse button is displayed during snapshots. The logos in the previous version where somewhat broken.

Contribution License Agreement

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

@snyk-io
Copy link
Contributor

snyk-io bot commented Aug 26, 2025

🎉 Snyk checks have passed. No issues have been found so far.

security/snyk check is complete. No issues have been found. (View Details)

license/snyk check is complete. No issues have been found. (View Details)

@github-actions
Copy link
Contributor

github-actions bot commented Aug 26, 2025

✅ PR preview is ready!

Name Link
📦 Wheel file https://core-previews.s3-us-west-2.amazonaws.com/pr-12329/streamlit-1.48.1-py3-none-any.whl
🕹️ Preview app pr-12329.streamlit.app (☁️ Deploy here if not accessible)

@lukasmasuch lukasmasuch added security-assessment-completed Security assessment has been completed for PR change:bugfix PR contains bug fix implementation impact:users PR changes affect end users labels Aug 26, 2025
@sfc-gh-lmasuch sfc-gh-lmasuch requested a review from Copilot August 26, 2025 17:53
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR fixes a UI layout issue where the logo was pushing out the collapse button in the Streamlit sidebar. The fix ensures proper positioning and visibility of both elements.

  • Added CSS objectPosition: "left" to prevent logo overflow
  • Wrapped the logo component in a div container for better layout control
  • Enhanced E2E tests to verify collapse button visibility alongside logo display

Reviewed Changes

Copilot reviewed 3 out of 17 changed files in this pull request and generated 1 comment.

File Description
frontend/app/src/components/Sidebar/styled-components.ts Added objectPosition: "left" CSS property to logo styling
frontend/app/src/components/Logo/LogoComponent.tsx Wrapped logo element in a div container
e2e_playwright/st_logo_test.py Added hover interactions and collapse button visibility checks to all logo tests

// handle the width in all cases. It already gets wrapped via a
// link element (<a>) above when link is provided.
// https://github.com/streamlit/streamlit/issues/12326
return <div>{logo}</div>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not entirely sure why having this wrapped into another element works fine, but it's probably related to some specific aspect of the surrounding flex box and how max-width is applied to the image.

@lukasmasuch lukasmasuch marked this pull request as ready for review August 26, 2025 18:57
Copy link
Collaborator

@mayagbarnes mayagbarnes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍🏼

@lukasmasuch lukasmasuch merged commit 3fbf974 into develop Aug 28, 2025
37 checks passed
@lukasmasuch lukasmasuch deleted the fix/logo-sizing branch August 28, 2025 20:44
lukasmasuch added a commit that referenced this pull request Aug 29, 2025
## Describe your changes

Fixes an issue with the logo pushing out the collapse button. This was
already working fine when a link was set -> the reason is that the
additional wrapping leads to a more correct calculation of the actual
available width. I think this is also solvable differently with some CSS
stuff, but I think its ok to just wrap the logo in a div when its not
already wrapped into a `a` element.

## GitHub Issue Link (if applicable)

- Closes #12326

## Testing Plan

- Updated the e2e tests to ensure that the collapse button is displayed
during snapshots. The logos in the previous version where somewhat
broken.

---

**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

change:bugfix PR contains bug fix implementation impact:users PR changes affect end users security-assessment-completed Security assessment has been completed for PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

st.logo can push minimise button outside sidebar

4 participants