fix #9339: Altair chart left side cut off on initial load if legend by goncalossmartins · Pull Request #10939 · streamlit/streamlit

goncalossmartins · 2025-03-27T13:53:57Z

is on bottom, only visible after rerun

Fix issue where Altair charts with legends set to None were cut
off on initial load and only rendered correctly after rerun (initially
thought to be related to the orientation of the legend).

Patch applies to the 'color' channel: when title is None, it is
replaced with a single space to prevent layout issues.

Changes Made

vega_charts.py: added _patch_null_legend_titles method
and a call for it in _prepare_vega_lite_spec method before return

GitHub Issue Link

Altair chart left side cut off on initial load if legend is on bottom, only visible after rerun #9339

Testing Plan

The effect of this bug is visual, either way, the previously existing
tests already verify the strength and integrity of the new code added.
To test this manually, run this code:

import streamlit as st
import pandas as pd
import altair as alt


st.title("Altair Chart cut-off bug")


df = pd.DataFrame(
    {"x": [1, 2, 3, 4], "y": [1, 2, 3, 4], "category": ["A", "B", "C", "D"]}
)

chart = (
    alt.Chart(df)
    .mark_line(point=True)
    .encode(
        x=alt.X("x", title="Date"),
        y=alt.Y("y:Q", title="Legend Value"),
        color=alt.Color("category:N", title="Category").legend(
            orient="bottom", title=None
        ),
    )
)

st.write("use_container_width=True")
st.altair_chart(chart, use_container_width=True)

# THE REST OF THIS JUST SHOWS MORE EXAMPLES

st.write("No bottom legend")
# This one renders properly initially
chart = (
    alt.Chart(df)
    .mark_line(point=True)
    .encode(
        x=alt.X("x", title="Date"),
        y=alt.Y("y:Q", title="Legend Value"),
        color=alt.Color("category:N", title="Category"),
    )
)

st.altair_chart(chart, use_container_width=True)


st.button("Rerender")

Contribution License Agreement

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

… legend is on bottom, only visible after rerun Fix issue where Altair charts with legends set to None were cut off on initial load and only rendered correctly after rerun. Patch applies to the 'color' channel: when title is None, it is replaced with a single space to prevent layout issues.

snyk-io · 2025-03-27T13:54:08Z

🎉 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)

sfc-gh-bnisco

Thank you for your contribution! I left a comment inline about docstring improvements. Additionally, we should get some new test coverage for this. I'd suggest adding some unit tests in lib/tests/streamlit/elements/vega_charts_test.py as well as some snapshot tests in e2e_playwright/st_altair_chart.py+e2e_playwright/st_altair_chart_test.py

sfc-gh-bnisco · 2025-03-27T16:40:35Z

lib/streamlit/elements/vega_charts.py



+def _patch_null_legend_titles(spec: VegaLiteSpec) -> None:
+    """Patches null legend titles in the 'color' channel of the spec."""


suggestion: Please update the docstrings with more information about why this change is necessary / what it fixes. It would be helpful to also reference issue #9339 here

Add test case in vega_charts_test.py to verify patching of null legend titles in the 'color' channel. Add E2E snapshot test in st_altair_chart.py/st_altair_chart_test.py to verify visual rendering of Altair charts when legend title is set to None and placed at the bottom.

sfc-gh-bnisco · 2025-03-28T18:29:03Z

@goncalossmartins Thank you again for your iterations on this! There seem to be some test failures in CI. I've outlined them below with some commentary that should hopefully help make solving these test failures easier:

For the snapshot failures, these are expected, since this PR adds new snapshots. Please update the snapshots and include them in this PR. We have instructions here in our wiki
Since this adds a new dataframe rendered in the st_altair_chart.py file, there are some e2e tests that fail with AssertionError: Locator expected to have count '10' Actual value: 11. This can be fixed by updating the values in test_altair_chart_displays_correctly to be 11
I see a Python unit test failure: FAILED tests/streamlit/elements/vega_charts_test.py::VegaLiteChartTest::test_altair_chart_patches_null_legend_title - TypeError: 'UndefinedType' object is not callable, which should be reproducible locally when running the test

Let me know if you have any questions, I'd be happy to help! Thank you!

goncalossmartins · 2025-04-02T13:07:57Z

@sfc-gh-bnisco Thank you very much for your help. I have finals coming up so I've been a bit away from the project for now, but really soon I will be over with them and I'll focus entirely on fixing the tests I provided. Thank you again for your comprehension!

corrected 'test_altair_chart_displays_correctly' count from 10 to 11. Added and updated snapshots related to new tests.

sfc-gh-bnisco

Seems like there are some failures in CI, along with an unexpected change in one of the snapshot tests. Can you please take a look? Thanks!

sfc-gh-bnisco · 2025-04-22T21:20:26Z

...right/__snapshots__/linux/st_altair_chart_test/st_altair_chart-tooltip_styling[chromium].png

issue: It seems unexpected that this tooltip is now missing a value, can you take a look to see what might be causing this?

This is likely due to my implementation and how when there is no value associated to a legend it is patched and a space is placed to replace it, therefore putting a space in the "category" spot. I will be looking into this, thank you.

goncalossmartins · 2025-04-24T11:57:49Z

@sfc-gh-bnisco Thank you for your help and sorry to bother but I'm having issues with updating the snapshots. I have corrected the problems raised before but I can't seem to update some of the snapshots back to their previous state (with the category on the second row), even though they are showing correctly in the tests. Only the chromium snapshot updated when running the update-snapshots script. How should I proceed?

Previously, every chart without a mention of a legend or a title would have the legend title patched to a single space, but was too broad and acted undesirably. Forced to only patch when the legend is purposefully 'None'. Fixed test snapshots accordingly.

sfc-gh-bnisco · 2025-04-25T22:33:35Z

Hi @goncalossmartins thank you for your message!

The reason it didn't update is because that script pulls the snapshot images from the last CI run. The CI run it was pulling from was probably didn't include your latest changes from your branch. I just re-ran the CI job with the latest changes. If you re-run the make command locally now, it should pull in the snapshot images from the latest run that includes your latest changes!

I am also seeing some Python Type Errors in the Python Unit Test jobs (example here). Please take a look at fixing that up! Thank you!

goncalossmartins · 2025-04-25T23:34:08Z

Hey @sfc-gh-bnisco! I'm truly sorry for those unit tests, they did not show up to me when I ran the make pytest script so I didn't know the errors were there, but I believe I've finally corrected these issues and there should be no more problems. Once more, thank you for your help.

S witched from a generic 'dict' type in the null title patching back to the 'VegaLiteSpec' believed to be the so lution to previous problems. Also added missing snapshots related to other E2E tests mistakingly deleted

sfc-gh-bnisco

Thank you for the iterations here @goncalossmartins , the snapshots are looking good! Seems like there is still an issue with one of the python unit tests. I left a comment inline. Please take a look and let me know if you have questions!

sfc-gh-bnisco · 2025-04-28T17:01:30Z

lib/tests/streamlit/elements/vega_charts_test.py

+            .encode(
+                x=alt.X("x", title="X Axis"),
+                y=alt.Y("y", title="Y Axis"),
+                color=alt.Color("category:N").legend(title=None),


@goncalossmartins Wanted to flag that this test is failing in CI. You should be able to see the job output here:

________ VegaLiteChartTest.test_altair_chart_patches_null_legend_title _________ self = <tests.streamlit.elements.vega_charts_test.VegaLiteChartTest testMethod=test_altair_chart_patches_null_legend_title> def test_altair_chart_patches_null_legend_title(self): """Test that legend.title=None is converted to ' ' in the 'color' channel of an Altair Chart.""" df = pd.DataFrame( { "x": [1, 2, 3], "y": [4, 5, 6], "category": ["A", "B", "C"], } ) chart = ( alt.Chart(df) .mark_line() .encode( x=alt.X("x", title="X Axis"), y=alt.Y("y", title="Y Axis"), > color=alt.Color("category:N").legend(title=None), ) ) E TypeError: 'UndefinedType' object is not callable df = x y category 0 1 4 A 1 2 5 B 2 3 6 C self = <tests.streamlit.elements.vega_charts_test.VegaLiteChartTest testMethod=test_altair_chart_patches_null_legend_title>

You should also be able to reproduce this error locally by running the tests: PYTHONPATH=lib pytest lib/tests/streamlit/elements/vega_charts_test.py. For more info, the docs on running tests are located here.

@goncalossmartins Very interesting! I agree with you, the tests are passing locally on Python 3.13 and 3.9 for me on MacOS, but it seems like this is consistently failing in CI on Python 3.9. I merged in the latest upstream develop since we've been making changes to our CI and linting systems and it looks like it is still failing but only on Python 3.9.

Given the error message, I wonder if changing the syntax in the test might help?

color=alt.Color("category:N", legend={"title": None}),

goncalossmartins · 2025-04-30T10:36:42Z

Hi @sfc-gh-bnisco! Sorry to bother. I tried running the command you suggested and I pass all the tests with only one warning, can you confirm this? On the other hand I have since started failing another unit test (ConfigTest.test_server_headless) do you know if this is expected? I don't know how my code affects it.

sfc-gh-bnisco · 2025-06-02T18:29:48Z

Hi @goncalossmartins Thanks again for making some more updates here! We've been doing a lot of work to update our linting rules and the code here is currently failing on some of the latest rules. Would you be able to rebase/merge develop into this branch and make some updates so that they pass the new lint rules? Let me know if you have questions, thank you!

goncalossmartins · 2025-06-04T09:36:41Z

Hey @sfc-gh-bnisco! Sorry to bother, how do I attack this security-assessment problem?

sfc-gh-bnisco · 2025-06-04T16:31:19Z

Thank you for all the iterations @goncalossmartins , I've added the necessary tag in order to make that CI check pass!

goncalossmartins · 2025-06-06T16:48:51Z

It was great working with you @sfc-gh-bnisco, have a good day!

sfc-gh-bnisco reviewed Mar 27, 2025

View reviewed changes

sfc-gh-bnisco added feature:st.altair_chart Related to the `st.altair_chart` element change:bugfix PR contains bug fix implementation impact:users PR changes affect end users labels Mar 27, 2025

sfc-gh-bnisco self-assigned this Apr 2, 2025

tests streamlit#9339: Unit and Snapshot tests fix for PR streamlit#10939

a5c1e8b

corrected 'test_altair_chart_displays_correctly' count from 10 to 11. Added and updated snapshots related to new tests.

sfc-gh-bnisco reviewed Apr 22, 2025

View reviewed changes

fix streamlit#9339: Addressing failed unit tests

2c61717

S witched from a generic 'dict' type in the null title patching back to the 'VegaLiteSpec' believed to be the so lution to previous problems. Also added missing snapshots related to other E2E tests mistakingly deleted

sfc-gh-bnisco reviewed Apr 28, 2025

View reviewed changes

sfc-gh-bnisco and others added 2 commits April 30, 2025 08:01

Merge branch 'develop' into bug_fix

46ad6b3

Syntax change in null legend title patching test

adbb8f0

tests streamlit#9339: Linting fixes

be5fab6

sfc-gh-bnisco approved these changes Jun 4, 2025

View reviewed changes

sfc-gh-bnisco added the security-assessment-completed label Jun 4, 2025

sfc-gh-bnisco merged commit d820367 into streamlit:develop Jun 4, 2025
36 of 37 checks passed



		def _patch_null_legend_titles(spec: VegaLiteSpec) -> None:
		"""Patches null legend titles in the 'color' channel of the spec."""

Conversation

goncalossmartins commented Mar 27, 2025

Changes Made

GitHub Issue Link

Testing Plan

Uh oh!

snyk-io bot commented Mar 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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

Uh oh!

sfc-gh-bnisco left a comment

Choose a reason for hiding this comment

Uh oh!

sfc-gh-bnisco Mar 27, 2025

Choose a reason for hiding this comment

Uh oh!

sfc-gh-bnisco commented Mar 28, 2025

Uh oh!

goncalossmartins commented Apr 2, 2025

Uh oh!

sfc-gh-bnisco left a comment

Choose a reason for hiding this comment

Uh oh!

sfc-gh-bnisco Apr 22, 2025

Choose a reason for hiding this comment

Uh oh!

goncalossmartins Apr 23, 2025

Choose a reason for hiding this comment

Uh oh!

goncalossmartins commented Apr 24, 2025

Uh oh!

sfc-gh-bnisco commented Apr 25, 2025

Uh oh!

goncalossmartins commented Apr 25, 2025

Uh oh!

sfc-gh-bnisco left a comment

Choose a reason for hiding this comment

Uh oh!

sfc-gh-bnisco Apr 28, 2025

Choose a reason for hiding this comment

Uh oh!

sfc-gh-bnisco Apr 30, 2025

Choose a reason for hiding this comment

Uh oh!

goncalossmartins commented Apr 30, 2025

Uh oh!

sfc-gh-bnisco commented Jun 2, 2025

Uh oh!

goncalossmartins commented Jun 4, 2025

Uh oh!

sfc-gh-bnisco commented Jun 4, 2025

Uh oh!

Uh oh!

goncalossmartins commented Jun 6, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

snyk-io bot commented Mar 27, 2025 •

edited

Loading