Skip to content

Conversation

@kmcgrady
Copy link
Collaborator

@kmcgrady kmcgrady commented Jan 5, 2023

📚 Context

We fixed sliders to adjust the label if it appears outside of the screen, but it fails for ranges where there are two thumb values. This change allows for us to adjust.

  • What kind of change does this PR introduce?

    • Bugfix

🧠 Description of Changes

  • This is a visible (user-facing) change

Revised:

image

Current:

image

🧪 Testing Done

  • Screenshots included
  • Added/Updated e2e tests

🌐 References


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 kmcgrady added the security-assessment-completed Security assessment has been completed for PR label Jan 5, 2023
@kmcgrady kmcgrady marked this pull request as ready for review January 5, 2023 07:22
@mayagbarnes mayagbarnes self-assigned this Jan 5, 2023
Comment on lines 99 to 101
this.thumbValueAlignment(
this.thumbRef[i].current,
this.thumbValueRef[i].current
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't believe this second call to this.thumbValueAlignment is necessary.
Did some console.log & manual testing to verify that the for loop iterates over both thumb value instances for the range, and removing does not change the alignment behavior.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

🤦 Yep! you are right. I just removed it. Great catch!

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

@kmcgrady kmcgrady merged commit b2baa76 into streamlit:develop Jan 6, 2023
tconkling added a commit that referenced this pull request Jan 6, 2023
* develop:
  Fix slider overlap values for ranges (#5913)
  Update Minor devDependencies - Part B (#5914)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

security-assessment-completed Security assessment has been completed for PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Left-side label of select_slider widget overflows when in 'range' mode

2 participants