add format option for slider widget and return texts for display values#154
Conversation
frontend/src/lib/widgetTheme.ts
Outdated
| color: $disabled ? gray : primary, | ||
| top: "-22px", | ||
| position: "absolute" as "absolute", | ||
| whiteSpace: "nowrap" as "nowrap", |
There was a problem hiding this comment.
Is the as "nowrap" required? Seems quite odd.
There was a problem hiding this comment.
Yes, is required, but the right way is: whiteSpace: "nowrap",
tvst
left a comment
tvst
left a comment
There was a problem hiding this comment.
Approved after comments are addressed.
If you feel like you were unable to satisfactorily address a comment, ping me on Slack and let's chat about solutions.
| import { debounce } from "lib/utils" | ||
| import { WidgetStateManager } from "lib/WidgetStateManager" | ||
| import { sliderOverrides } from "lib/widgetTheme" | ||
| var sprintf = require("sprintf-js").sprintf |
There was a problem hiding this comment.
Please change to a normal import. If not possible, add comment explaining why.
| overrides={sliderOverrides} | ||
| overrides={{ | ||
| ...sliderOverrides, | ||
| // @Dani:here is doc about how override subcomponents: https://baseweb.design/components/slider/ |
There was a problem hiding this comment.
This comment is not necessary, please remove.
(What I was asking for before was to put a comment saying where you took the code from, but after reading that page I see your code is different enough from the source that this isn't needed.)
frontend/src/lib/widgetTheme.ts
Outdated
| position: "absolute" as "absolute", | ||
| whiteSpace: "nowrap", | ||
| backgroundColor: "transparent", | ||
| lineHeight: 1.6, |
There was a problem hiding this comment.
Use lineHeightBase instead. Same below.
frontend/src/lib/widgetTheme.ts
Outdated
| paddingBottom: smallTextMargin, | ||
| color: $disabled ? gray : primary, | ||
| top: "-22px", | ||
| position: "absolute" as "absolute", |
There was a problem hiding this comment.
Continuing the conversation from here: why is the as "absolute" needed? What kind of error do you get if this isn't included? I'm having a hard time understanding what's happening behind the scenes in the TS compiler...
There was a problem hiding this comment.
It's a TS problem, I don't know exactly why. I solved it casting the object:
const tickBarItemStyle = sliderOverrides.TickBarItem.style as React.CSSProperties
| "react-virtualized": "^9.21.0", | ||
| "reactstrap": "^8.0.1", | ||
| "recharts": "^1.6.2", | ||
| "sprintf-js": "^1.1.2", |
There was a problem hiding this comment.
Don't forget to regenerated the NOTICES file:
make notices
There was a problem hiding this comment.
the make notices command is wrong, I will update the make file, the right command is 'yarn licenses`.
Do you want to change the make verb? : make licenses maybe?
lib/streamlit/DeltaGenerator.py
Outdated
| step : int/float | ||
| The stepping interval. | ||
| Defaults to 1 if the value is an int, 0.01 otherwise. | ||
| format : str |
There was a problem hiding this comment.
And, actually, please change the other arguments above that accept None.
| if all_ints: | ||
| format = "%d" | ||
| else: | ||
| format = "%0.2f" |
There was a problem hiding this comment.
Add a comment here saying something like:
It would be great if we could guess the number of decimal places from the step`argument, but this would only be meaningful if step were a decimal. As a possible improvement we could make this function accept decimals and/or use some heuristics for floats.
# By Jonathan Rhone (3) and others # Via GitHub * develop: Text changes in ☰ menu and several CLI prompts (streamlit#180) add format option for slider widget and return texts for display values (streamlit#154) Merge ForwardMsg caching into develop (streamlit#178) e2e test for magic (streamlit#172) Bart example update (streamlit#179) Pedantic CircleCI cleanup (streamlit#174) Fix overwriting elements in the sidebar (streamlit#181) update vega lite chart snapshot (streamlit#183) Stop react-markdown from converting "[foo]" to a link. (streamlit#151) Issue 1204: Graphs using Vega-Lite (streamlit#56) Removing uber demo from streamlit repo (streamlit#159) # Conflicts: # lib/streamlit/DeltaGenerator.py
2 participants
https://github.com/streamlit/streamlit-old-private/issues/1100
*Added new field to Slider proto definition:
-string format = 7;
*Overridden ThumbValue and Tick subcomponents from UISlider component