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

Sliders should show current value [regression] [Baseweb] #2699

Closed
nthmost opened this issue Feb 2, 2021 · 1 comment · Fixed by #2724
Closed

Sliders should show current value [regression] [Baseweb] #2699

nthmost opened this issue Feb 2, 2021 · 1 comment · Fixed by #2724
Labels
area:widgets type:bug Something isn't working type:regression This bug is a regression

Comments

@nthmost
Copy link
Contributor

nthmost commented Feb 2, 2021

Summary

(via @tvst: )

Our sliders right now require you to hover in order to see the selected value. This makes it really hard to understand what the user selected. I reported this before, but I just spent some time debugging my app thinking it was broken because I was reading the wrong slider value. Frustrating.

I understand this is the new behavior of sliders in Base Web, but we have alternatives:

1. Roll back Base Web to a previous version

This is the preferable solution in order to get this fix out ASAP. Even if we decide it's only a temporary solution.
  1. Try to find a solution using the latest Base Web

  2. Copy/paste the old Baseweb slider into our own repo and modify it there. Their slider is based on another library, btw (I forget which), so maybe we should just use that library directly instead?

Is this a regression?

yes

Debug info

  • Streamlit version: 0.75-special
@nthmost nthmost added type:bug Something isn't working type:regression This bug is a regression area:widgets status:needs-triage Has not been triaged by the Streamlit team and removed status:needs-triage Has not been triaged by the Streamlit team labels Feb 2, 2021
@karriebear
Copy link
Contributor

Here's an approach from @bh-streamlit: https://codesandbox.io/s/base-web-overrides-usage-forked-id7p3?file=/src/example.js:1013-1053 that would probably work best.

Essentially the concept is to pass the value as the child of the thumb so we would create our own value instead of relying on baseweb's display value. Quite brilliant!

We would just need to get the styling in and make sure it doesn't conflict with baseweb's logic. From an initial look, I think its fine.

kantuni added a commit that referenced this issue Feb 4, 2021
kantuni added a commit that referenced this issue Feb 10, 2021
* Close #2699

* Override Thumb subcomponent completely

* Fixed linting errors

Co-authored-by: Ken McGrady <ken@streamlit.io>
CFrez pushed a commit to CFrez/streamlit that referenced this issue Feb 18, 2021
* Close streamlit#2699

* Override Thumb subcomponent completely

* Fixed linting errors

Co-authored-by: Ken McGrady <ken@streamlit.io>
schaumb pushed a commit to FloWide/streamlit that referenced this issue Feb 22, 2021
* Close streamlit#2699

* Override Thumb subcomponent completely

* Fixed linting errors

Co-authored-by: Ken McGrady <ken@streamlit.io>
kmcgrady pushed a commit that referenced this issue Feb 24, 2021
* created conditional rendering of controls & tests

* reorganized and added tests

* removed accidently added unneeded import

* alter flex, padding and gap to align columns

* adjusted spacing since not duplicated

* alter flex, padding and gap to align columns

* adjusted spacing since not duplicated

* Fix checkbox spacing (#2738)

* remove extra padding

* remove snapshot completely

* add snapshot back in

* remove snapshots

* remove rest of snapshots

* add snapshots back in

* Autofocus "clear cache" button (#2739)

* autofocus clear cache button

* add test

* update test

* fix lint

* `allow_multiple_files` -> `accept_multiple_files` (#2761)

* Update component-template submodule to latest (#2767)

Point our `component-template` submodule to the latest commit in that repo.

(Among other things, this will de-dupe a bunch of the `component-lib` code that used to live in `component-template` before being pulled into its own npm package.)

* Slider thumb values are always visible (#2724)

* Close #2699

* Override Thumb subcomponent completely

* Fixed linting errors

Co-authored-by: Ken McGrady <ken@streamlit.io>

* Create base color picker for use with API and internally (#2778)

* Copy to shared and cleanup

* Cleanup

* remove comment

* config: client.showTracebacks (#2770)

Adds a new config option, `"client.showTracebacks"`. By default, it's `True`.

If the option is set to `False`, then uncaught exceptions in a Streamlit app will result in a generic "Something went wrong!" warning in the browser, rather than the exception and its traceback

This logic happens in the `error_util.handle_uncaught_app_exception` function, rather than directly within ScriptRunner.

`scriptrunner_test`, `caching_test`, and `streamlit_test` have new tests that verify the right thing happens when exceptions are thrown in different circumstances with this config option turned on and off.

Closes #1032

* Shared selectbox (#2795)

* Remove nonexistent elements from widget state (#2760)

* remove nonexistent elements from widget states

* fix typo

* add test

* Fix datetime timezone handling in data frames (#2784)

* save changes

* remove redundant change

* fix tests

* fix quotes

* add tests and other fixes

* fix lint

* update protobufs and tests

* add e2e test

* fix quotes

* update test

* Revert "removed accidently added unneeded import"

This reverts commit f886adc.

* Revert "reorganized and added tests"

This reverts commit d3632d9.

* Revert "created conditional rendering of controls & tests"

This reverts commit 7c4400e.

* updated tests

* fixed lint

* correct percentages in spec for test

* update st_image column image

* added comment to HoizontalBlock

* added Issue/PR to comment

Co-authored-by: bh-streamlit <76131205+bh-streamlit@users.noreply.github.com>
Co-authored-by: Simon Biggs <me@simonbiggs.net>
Co-authored-by: Tim Conkling <tconkling@gmail.com>
Co-authored-by: Henrikh Kantuni <henrikh.kantuni@gmail.com>
Co-authored-by: Ken McGrady <ken@streamlit.io>
Co-authored-by: karrie <karrie@streamlit.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:widgets type:bug Something isn't working type:regression This bug is a regression
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants