-
Notifications
You must be signed in to change notification settings - Fork 14.5k
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
Fixes to the play slider #5675
Fixes to the play slider #5675
Changes from 3 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,8 +1,7 @@ | ||
.play-slider { | ||
position: absolute; | ||
bottom: -16px; | ||
height: 20px; | ||
width: 100%; | ||
width: 90%; | ||
} | ||
|
||
.slider-selection { | ||
|
@@ -21,3 +20,7 @@ | |
color: #b3b3b3; | ||
margin-right: 5px; | ||
} | ||
|
||
div.slider > div.tooltip.tooltip-main.top.in { | ||
margin-left: 0 !important; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is there a way to override without using There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we can use a more specific selector, let me take a look. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @kristw, the 3rd-party library |
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -21,6 +21,7 @@ const propTypes = { | |
orientation: PropTypes.oneOf(['horizontal', 'vertical']), | ||
reversed: PropTypes.bool, | ||
disabled: PropTypes.bool, | ||
range: PropTypes.bool, | ||
}; | ||
|
||
const defaultProps = { | ||
|
@@ -30,6 +31,7 @@ const defaultProps = { | |
orientation: 'horizontal', | ||
reversed: false, | ||
disabled: false, | ||
range: true, | ||
}; | ||
|
||
export default class PlaySlider extends React.PureComponent { | ||
|
@@ -87,7 +89,11 @@ export default class PlaySlider extends React.PureComponent { | |
if (this.props.disabled) { | ||
return; | ||
} | ||
let values = this.props.values.map(value => value + this.increment); | ||
let values = this.props.values; | ||
if (!Array.isArray(values)) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The code in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, good point, will fix. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It can be confusing if const { start, end, step, values, disabled } = this.props;
if (disabled) {
return;
}
const newValues = (Array.isArray(values) ? values : [values, values + step])
.map(v => v + this.increment);
// What does cr stand for?
const cr = (newValues[1] > end) ? (newValues[0] - start) : 0;
this.props.onChange(cr === 0 ? newValues : newValues.map(v => v - cr)); There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. CR is carriage return — how much the controls move back once they hit the end of the slider. I'll make the code more clear, thanks! |
||
values = [values, values + this.props.step]; | ||
} | ||
values = values.map(value => value + this.increment); | ||
if (values[1] > this.props.end) { | ||
const cr = values[0] - this.props.start; | ||
values = values.map(value => value - cr); | ||
|
@@ -108,6 +114,7 @@ export default class PlaySlider extends React.PureComponent { | |
return parts.map(value => (new Date(value)).toUTCString()).join(' : '); | ||
} | ||
render() { | ||
const { start, end, step, orientation, reversed, disabled, range, values } = this.props; | ||
return ( | ||
<Row className="play-slider"> | ||
<Col md={1} className="padded"> | ||
|
@@ -116,15 +123,16 @@ export default class PlaySlider extends React.PureComponent { | |
</Col> | ||
<Col md={11} className="padded"> | ||
<ReactBootstrapSlider | ||
value={this.props.values} | ||
value={range ? values : values[0]} | ||
range={range} | ||
formatter={this.formatter} | ||
change={this.onChange} | ||
min={this.props.start} | ||
max={this.props.end} | ||
step={this.props.step} | ||
orientation={this.props.orientation} | ||
reversed={this.props.reversed} | ||
disabled={this.props.disabled ? 'disabled' : 'enabled'} | ||
min={start} | ||
max={end} | ||
step={step} | ||
orientation={orientation} | ||
reversed={reversed} | ||
disabled={disabled ? 'disabled' : 'enabled'} | ||
/> | ||
</Col> | ||
</Row> | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Try wrapping PlaySlider in
<div style="position: relative">
It will be better to have the slider at full width rather than 90%
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1