Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 16 additions & 4 deletions src/Slider.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,21 @@ import * as SliderConstants from './constants/SliderConstants';
import linear from './algorithms/linear';

function getClassName(props) {
const orientation = props.orientation === 'vertical'
let classNames = ['rheostat'];

classNames.push(props.orientation === 'vertical'
? 'rheostat-vertical'
: 'rheostat-horizontal';
: 'rheostat-horizontal');
Copy link
Collaborator

Choose a reason for hiding this comment

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

the ); needs to be on a line by itself; perhaps a cleaner option is classNames.push(orientation), or const classNames = ['rheostat', orientation] after the ternary?

Copy link
Author

Choose a reason for hiding this comment

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

fixed


if (props.disabled) {
classNames.push('rheostat-disabled');
}

return ['rheostat', orientation].concat(props.className.split(' ')).join(' ');
if (props.className) {
classNames = [...classNames, ...props.className.split(' ')];
Copy link
Collaborator

Choose a reason for hiding this comment

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

classNames.push(...props.className.split(' ')), but this should probably be using the cx package instead of manually splitting :-/

}

return classNames.join(' ');
}

const has = Object.prototype.hasOwnProperty;
Expand Down Expand Up @@ -197,7 +207,9 @@ class Rheostat extends React.Component {

const willBeDisabled = nextProps.disabled && !disabled;

if (orientationChanged) {
const disabledChanged = nextProps.disabled !== disabled;

if (orientationChanged || disabledChanged) {
this.setState({
className: getClassName(nextProps),
});
Expand Down
28 changes: 28 additions & 0 deletions test/slider-test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,15 @@ describeWithDOM('<Slider />', () => {
assert.isTrue(pitRender.calledOnce, 'one pit was rendered only once');
});

it('should render the disabled state', () => {
const slider = mount(<Slider disabled />);
assert.include(
slider.state('className'),
'rheostat-disabled',
'the disabled class was rendered',
);
});

it('should not throw react errors on disabled', () => {
const slider = mount(<Slider />);
slider.setProps({ disabled: true });
Expand Down Expand Up @@ -127,6 +136,25 @@ describeWithDOM('<Slider />', () => {
);
});

it('should re-evaluate the disabled state when props change', () => {
const slider = mount(<Slider />);
assert.isFalse(slider.props().disabled, 'slider is not disabled');
assert.notInclude(
slider.state('className'),
'rheostat-disabled',
'cached class has not disabled',
);

slider.setProps({ disabled: true });

assert.isTrue(slider.props().disabled, 'slider was disabled');
assert.include(
slider.state('className'),
'rheostat-disabled',
'the cached classes were updated',
);
});

it('should not call onChange twice if values are the same as what is in state', () => {
const onChange = sinon.spy();
const slider = mount(<Slider onChange={onChange} values={[0]} />);
Expand Down