Slider Theme #257
Slider Theme #257
Conversation
Codecov Report
@@ Coverage Diff @@
## master #257 +/- ##
==========================================
- Coverage 98.95% 98.78% -0.17%
==========================================
Files 24 24
Lines 1429 1476 +47
Branches 413 434 +21
==========================================
+ Hits 1414 1458 +44
- Partials 15 18 +3
Continue to review full report at Codecov.
|
src/slider/Slider.ts
Outdated
| classes: this.classes(css.output), | ||
| for: this._inputId + '' | ||
| classes: this.classes(css.output, outputIsTooltip ? css.outputTooltip : null), | ||
| for: this._inputId + '', |
There was a problem hiding this comment.
not a big fan of the + '' approach for casting to a string. Would it not be better to use a template string? ie.
for: `${this._inputId}`
There was a problem hiding this comment.
@smhigley: Looks like @tomdye is right, template strings are fastest now (previously String concatenation was still fastest; something must've been improved.)
| // output styles | ||
| let outputStyles: { left?: string; top?: string } = {}; | ||
| if (outputIsTooltip) { | ||
| outputStyles = vertical ? { top: ((max - value) / max * 100) + '%' } : { left: (value / max * 100) + '%' }; |
There was a problem hiding this comment.
Can you use a template string for this? Might look cleaner than + '%'
Type: feature
The following has been addressed in the PR:
Description:
Adds base theme for the slider widget. Modified the widget to include a boolean property to create a tooltip tied to the slider handle.
Also added body padding to examples.