Skip to content

[Fix] Disabled widgets show border when showWidgetBorder=true #12949

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.

Sign up for GitHub

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

Jump to bottom
Merged
mayagbarnes merged 11 commits into from
Nov 8, 2025
Merged

[Fix] Disabled widgets show border when showWidgetBorder=true #12949

mayagbarnes merged 11 commits into from
Nov 8, 2025

Conversation

@mayagbarnes
Copy link
Collaborator

@mayagbarnes mayagbarnes commented Nov 5, 2025 •
edited
Loading

Describe your changes

Fix widget border display when showWidgetBorder=true for disabled widgets. The following were updated:

  • st.selectbox
  • st.multiselect
  • st.date_input
  • st.time_input
  • st.text_input
  • st.text_area

Current: only some widgets are correct (ex: audio input)
Screenshot 2025-11-05 at 12 16 33 p m

After:
Screenshot 2025-11-05 at 12 15 41 p m

Testing Plan

  • JS Unit Tests: ❌ Baseweb dynamic styling can't be picked up in unit tests
  • E2E Tests: ✅ Existing snapshots should remain the same, added e2e snapshot test
  • Manual Testing: ✅

@mayagbarnes mayagbarnes 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 Nov 5, 2025
@snyk-io
Copy link
Contributor

snyk-io bot commented Nov 5, 2025 •
edited
Loading

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

Status Scanner Critical High Medium Low Total (0)
✅ Licenses 0 0 0 0 0 issues
✅ Open Source Security 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

@github-actions
Copy link
Contributor

github-actions bot commented Nov 5, 2025 •
edited
Loading

✅ PR preview is ready!

Name Link
📦 Wheel file https://core-previews.s3-us-west-2.amazonaws.com/pr-12949/streamlit-1.51.0-py3-none-any.whl
📦 @streamlit/component-v2-lib Download from artifacts
🕹️ Preview app pr-12949.streamlit.app (☁️ Deploy here if not accessible)

@mayagbarnes mayagbarnes requested a review from Copilot November 5, 2025 22:46
@mayagbarnes mayagbarnes marked this pull request as ready for review November 5, 2025 22:46
Copilot AI reviewed Nov 5, 2025
View reviewed changes
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 pull request adds dynamic border color styling to input widgets based on their focus state. When an input is focused, its border changes to the primary theme color, providing better visual feedback to users.

Key changes:

  • Converted static style objects to style functions that receive $isFocused state
  • Added dynamic border color logic that switches between widget border color and primary color on focus
  • Applied this pattern consistently across multiple input widget components

Reviewed Changes

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

Show a summary per file
File Description
TimeInput.tsx Added focus-based border color styling to the ControlContainer override
TextInput.tsx Added focus-based border color styling to the Root override
TextArea.tsx Added focus-based border color styling to the Root override
Multiselect.tsx Added focus-based border color styling to the ControlContainer override
DateInput.tsx Added focus-based border color styling to the Root override of the Input component
Selectbox.tsx Added focus-based border color styling to the ControlContainer override

sfc-gh-bnisco
sfc-gh-bnisco reviewed Nov 6, 2025
View reviewed changes
Copy link
Collaborator

@sfc-gh-bnisco sfc-gh-bnisco left a comment

Choose a reason for hiding this comment

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

Thanks for the screenshots, they look great! A few broader questions/suggestions:

  1. suggestion: This should have some tests for this functionality.
  2. question: Are there any other widgets that need to be updated or do we expect this PR to encompass all widgets that require this logic?
  3. question: Looking forward, if we were to introduce a new widget that also needs this visual styling logic, how can we ensure it gets this? Ideally, we'd have some type of automated system/reusable component that enforces this so we aren't reliant on internal knowledge of expected behavior.

@mayagbarnes
Copy link
Collaborator Author

mayagbarnes commented Nov 7, 2025

  1. suggestion: This should have some tests for this functionality.

I kept trying to find a way to cover this in unit tests, but begrudgingly have to do e2e snapshot test (due to dynamic styling of these Baseweb components)

  1. question: Are there any other widgets that need to be updated or do we expect this PR to encompass all widgets that require this logic?

This should cover the necessary updates, the other widgets (st.chat_input, st.audio_input, etc.) were already correct.

  1. question: Looking forward, if we were to introduce a new widget that also needs this visual styling logic, how can we ensure it gets this? Ideally, we'd have some type of automated system/reusable component that enforces this so we aren't reliant on internal knowledge of expected behavior.

Makes sense, at this point I don't have a clear idea of how to implement an overarching component given some of these are baseweb components and others our own styled components (but not all are divs for example). I'll try to revisit this.

sfc-gh-bnisco
sfc-gh-bnisco approved these changes Nov 7, 2025
View reviewed changes
Copy link
Collaborator

@sfc-gh-bnisco sfc-gh-bnisco left a comment

Choose a reason for hiding this comment

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

Thanks for the revisions and for your thoughtful answers to my questions/suggestions. LGTM!

frontend/lib/src/components/shared/Base/styled-components.ts Outdated
Comment on lines 31 to 32
// Helper function to handle the border color for baseweb input widgets
// @see Selectbox, Multiselect, DateInput, TimeInput, TextInput, TextArea, NumberInput
Copy link
Collaborator

@sfc-gh-bnisco sfc-gh-bnisco Nov 7, 2025

Choose a reason for hiding this comment

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

nit: These should be formatted as JSDoc comments for better IDE/tooling integration

@github-actions
Copy link
Contributor

github-actions bot commented Nov 8, 2025

📉 Frontend coverage change detected

The frontend unit test (vitest) coverage has decreased by 0.0500%

  • Current PR: 85.9700% (50556 lines, 7089 missed)
  • Latest develop: 86.0200% (50497 lines, 7059 missed)

💡 Consider adding more unit tests to maintain or improve coverage.

📊 View detailed coverage comparison

@mayagbarnes mayagbarnes merged commit b547005 into develop Nov 8, 2025
40 checks passed
@mayagbarnes mayagbarnes deleted the fix-widget-borders branch November 8, 2025 08:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@sfc-gh-bnisco sfc-gh-bnisco sfc-gh-bnisco approved these changes

Assignees

No one assigned

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

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@mayagbarnes @sfc-gh-bnisco