From c824064aa95890d60b0ac296c080e68021ff2288 Mon Sep 17 00:00:00 2001 From: davidchin Date: Thu, 26 Jan 2017 11:09:03 +1100 Subject: [PATCH] refator: Stop passing components in callbacks --- example/js/example-app.jsx | 14 +++---- react-input-range.d.ts | 2 +- src/js/input-range/input-range.jsx | 36 +++++------------- src/js/input-range/slider.jsx | 8 ++-- src/js/input-range/track.jsx | 2 +- test/input-range/input-range.spec.jsx | 55 ++++++++++++--------------- 6 files changed, 48 insertions(+), 69 deletions(-) diff --git a/example/js/example-app.jsx b/example/js/example-app.jsx index d6917de..2b653a7 100644 --- a/example/js/example-app.jsx +++ b/example/js/example-app.jsx @@ -32,43 +32,43 @@ export default class ExampleApp extends React.Component { this.handleValue5Change = this.handleValue5Change.bind(this); } - handleValueChange(component, value) { + handleValueChange(value) { this.setState({ value: value || 0, }); } - handleValue2Change(component, value) { + handleValue2Change(value) { this.setState({ value2: value || 0, }); } - handleValue3Change(component, value) { + handleValue3Change(value) { this.setState({ value3: value || 0, }); } - handleValue4Change(component, values) { + handleValue4Change(values) { this.setState({ value4: values, }); } - handleValue5Change(component, values) { + handleValue5Change(values) { this.setState({ value5: values, }); } - handleValue6Change(component, values) { + handleValue6Change(values) { this.setState({ value6: values, }); } - handleChangeComplete(component, value) { + handleChangeComplete(value) { console.log(value); } diff --git a/react-input-range.d.ts b/react-input-range.d.ts index 7411d90..a23f6fa 100644 --- a/react-input-range.d.ts +++ b/react-input-range.d.ts @@ -40,7 +40,7 @@ declare namespace ReactInputRange { maxValue?: number; minValue?: number; name?: string; - onChange: (element: this, value: (number | IRange)) => void; + onChange: (value: (number | IRange)) => void; onChangeComplete?: () => void; step?: number; value: number | IRange; diff --git a/src/js/input-range/input-range.jsx b/src/js/input-range/input-range.jsx index fc48fcc..778549f 100644 --- a/src/js/input-range/input-range.jsx +++ b/src/js/input-range/input-range.jsx @@ -159,19 +159,6 @@ export default class InputRange extends React.Component { return 'max'; } - /** - * Get the key name of a slider - * @param {Slider} slider - React component - * @return {string} Key name - */ - getKeyFromSlider(slider) { - if (slider === this.sliderMinNode) { - return 'min'; - } - - return 'max'; - } - /** * Get all slider keys of inputRange * @return {string[]} Key names @@ -286,9 +273,9 @@ export default class InputRange extends React.Component { } if (this.isMultiValue()) { - this.props.onChange(this, values); + this.props.onChange(values); } else { - this.props.onChange(this, values.max); + this.props.onChange(values.max); } } @@ -332,14 +319,13 @@ export default class InputRange extends React.Component { /** * Handle any mousemove event received by the slider * @param {SyntheticEvent} event - User event - * @param {Slider} slider - React component + * @param {string} key - Slider type */ - handleSliderMouseMove(event, slider) { + handleSliderMouseMove(event, key) { if (this.props.disabled) { return; } - const key = this.getKeyFromSlider(slider); const position = valueTransformer.positionFromEvent(event, this.getTrackClientRect()); requestAnimationFrame(() => this.updatePosition(key, position)); @@ -348,15 +334,14 @@ export default class InputRange extends React.Component { /** * Handle any keydown event received by the slider * @param {SyntheticEvent} event - User event + * @param {string} key - Slider type * @param {Slider} slider - React component */ - handleSliderKeyDown(event, slider) { + handleSliderKeyDown(event, key) { if (this.props.disabled) { return; } - const key = this.getKeyFromSlider(slider); - switch (event.keyCode) { case LEFT_ARROW: case DOWN_ARROW: @@ -378,10 +363,9 @@ export default class InputRange extends React.Component { /** * Handle any mousedown event received by the track * @param {SyntheticEvent} event - User event - * @param {Track} track - React component * @param {Point} position - Mousedown position */ - handleTrackMouseDown(event, track, position) { + handleTrackMouseDown(event, position) { if (this.props.disabled) { return; } @@ -411,7 +395,7 @@ export default class InputRange extends React.Component { } if (this.startValue !== this.props.value) { - this.props.onChangeComplete(this, this.props.value); + this.props.onChangeComplete(this.props.value); } this.startValue = null; @@ -525,11 +509,11 @@ export default class InputRange extends React.Component { /** * Get an array of hidden input HTML for rendering - * @return {?string[]} Array of HTML + * @return {string[]} Array of HTML */ renderHiddenInputs() { if (!this.props.name) { - return; + return []; } const isMultiValue = this.isMultiValue(); diff --git a/src/js/input-range/slider.jsx b/src/js/input-range/slider.jsx index 1d19507..986ce07 100644 --- a/src/js/input-range/slider.jsx +++ b/src/js/input-range/slider.jsx @@ -18,6 +18,7 @@ export default class Slider extends React.Component { * @property {Function} onSliderKeyDown * @property {Function} onSliderMouseMove * @property {Function} percentage + * @property {Function} type * @property {Function} value */ static get propTypes() { @@ -31,6 +32,7 @@ export default class Slider extends React.Component { onSliderKeyDown: React.PropTypes.func.isRequired, onSliderMouseMove: React.PropTypes.func.isRequired, percentage: React.PropTypes.number.isRequired, + type: React.PropTypes.string.isRequired, value: React.PropTypes.number.isRequired, }; } @@ -102,7 +104,7 @@ export default class Slider extends React.Component { * @param {SyntheticEvent} event - User event */ handleMouseMove(event) { - this.props.onSliderMouseMove(event, this); + this.props.onSliderMouseMove(event, this.props.type); } /** @@ -123,7 +125,7 @@ export default class Slider extends React.Component { * @param {SyntheticEvent} event - User event */ handleTouchMove(event) { - this.props.onSliderMouseMove(event, this); + this.props.onSliderMouseMove(event, this.props.type); } /** @@ -143,7 +145,7 @@ export default class Slider extends React.Component { * @param {SyntheticEvent} event - User event */ handleKeyDown(event) { - this.props.onSliderKeyDown(event, this); + this.props.onSliderKeyDown(event, this.props.type); } /** diff --git a/src/js/input-range/track.jsx b/src/js/input-range/track.jsx index a8ae03a..49cdbf4 100644 --- a/src/js/input-range/track.jsx +++ b/src/js/input-range/track.jsx @@ -66,7 +66,7 @@ export default class Track extends React.Component { y: 0, }; - this.props.onTrackMouseDown(event, this, position); + this.props.onTrackMouseDown(event, position); } /** diff --git a/test/input-range/input-range.spec.jsx b/test/input-range/input-range.spec.jsx index 301373f..c9afad4 100644 --- a/test/input-range/input-range.spec.jsx +++ b/test/input-range/input-range.spec.jsx @@ -78,7 +78,7 @@ describe('InputRange', () => { it('should call `onChange` callback', () => { inputRange.updateValues(newValues); - expect(onChange).toHaveBeenCalledWith(inputRange, newValues); + expect(onChange).toHaveBeenCalledWith(newValues); }); }); @@ -92,7 +92,7 @@ describe('InputRange', () => { it('should call `onChange` callback', () => { inputRange.updateValues(newValues); - expect(onChange).toHaveBeenCalledWith(inputRange, newValues.max); + expect(onChange).toHaveBeenCalledWith(newValues.max); }); }); }); @@ -219,12 +219,10 @@ describe('InputRange', () => { describe('handleSliderMouseMove', () => { let event; let requestAnimationFrame; - let slider; beforeEach(() => { spyOn(inputRange, 'updatePosition'); - slider = inputRange.sliderMaxNode; event = { clientX: 100, clientY: 200, @@ -241,7 +239,7 @@ describe('InputRange', () => { }); it('should set the position of a slider according to mouse event', () => { - inputRange.handleSliderMouseMove(event, slider); + inputRange.handleSliderMouseMove(event, 'max'); jasmine.clock().tick(0); expect(inputRange.updatePosition).toHaveBeenCalledWith('max', { x: 92, y: 0 }); @@ -250,7 +248,7 @@ describe('InputRange', () => { it('should not set the position of a slider if disabled', () => { inputRange = renderComponent(); spyOn(inputRange, 'updatePosition'); - inputRange.handleSliderMouseMove(event, slider); + inputRange.handleSliderMouseMove(event, 'max'); jasmine.clock().tick(0); expect(inputRange.updatePosition).not.toHaveBeenCalled(); @@ -258,14 +256,12 @@ describe('InputRange', () => { }); describe('handleSliderKeyDown', () => { - let slider; let event; describe('when pressing left arrow key', () => { beforeEach(() => { spyOn(inputRange, 'decrementValue'); - slider = inputRange.sliderMaxNode; event = { keyCode: 37, preventDefault: jasmine.createSpy('preventDefault'), @@ -273,13 +269,13 @@ describe('InputRange', () => { }); it('should decrement value', () => { - inputRange.handleSliderKeyDown(event, slider); + inputRange.handleSliderKeyDown(event, 'max'); expect(inputRange.decrementValue).toHaveBeenCalledWith('max'); }); it('should call event.preventDefault()', () => { - inputRange.handleSliderKeyDown(event, slider); + inputRange.handleSliderKeyDown(event, 'max'); expect(event.preventDefault).toHaveBeenCalledWith(); }); @@ -287,7 +283,7 @@ describe('InputRange', () => { it('should not decrement value if disabled', () => { inputRange = renderComponent(); spyOn(inputRange, 'decrementValue'); - inputRange.handleSliderKeyDown(event, slider); + inputRange.handleSliderKeyDown(event, 'max'); expect(inputRange.decrementValue).not.toHaveBeenCalled(); }); @@ -297,7 +293,6 @@ describe('InputRange', () => { beforeEach(() => { spyOn(inputRange, 'decrementValue'); - slider = inputRange.sliderMaxNode; event = { keyCode: 40, preventDefault: jasmine.createSpy('preventDefault'), @@ -305,13 +300,13 @@ describe('InputRange', () => { }); it('should decrement value', () => { - inputRange.handleSliderKeyDown(event, slider); + inputRange.handleSliderKeyDown(event, 'max'); expect(inputRange.decrementValue).toHaveBeenCalledWith('max'); }); it('should call event.preventDefault()', () => { - inputRange.handleSliderKeyDown(event, slider); + inputRange.handleSliderKeyDown(event, 'max'); expect(event.preventDefault).toHaveBeenCalledWith(); }); @@ -319,7 +314,7 @@ describe('InputRange', () => { it('should not decrement value if disabled', () => { inputRange = renderComponent(); spyOn(inputRange, 'decrementValue'); - inputRange.handleSliderKeyDown(event, slider); + inputRange.handleSliderKeyDown(event, 'max'); expect(inputRange.decrementValue).not.toHaveBeenCalled(); }); @@ -329,7 +324,6 @@ describe('InputRange', () => { beforeEach(() => { spyOn(inputRange, 'incrementValue'); - slider = inputRange.sliderMaxNode; event = { keyCode: 39, preventDefault: jasmine.createSpy('preventDefault'), @@ -337,13 +331,13 @@ describe('InputRange', () => { }); it('should increment value', () => { - inputRange.handleSliderKeyDown(event, slider); + inputRange.handleSliderKeyDown(event, 'max'); expect(inputRange.incrementValue).toHaveBeenCalledWith('max'); }); it('should call event.preventDefault()', () => { - inputRange.handleSliderKeyDown(event, slider); + inputRange.handleSliderKeyDown(event, 'max'); expect(event.preventDefault).toHaveBeenCalledWith(); }); @@ -351,7 +345,7 @@ describe('InputRange', () => { it('should not increment value if disabled', () => { inputRange = renderComponent(); spyOn(inputRange, 'incrementValue'); - inputRange.handleSliderKeyDown(event, slider); + inputRange.handleSliderKeyDown(event, 'max'); expect(inputRange.incrementValue).not.toHaveBeenCalled(); }); @@ -361,7 +355,6 @@ describe('InputRange', () => { beforeEach(() => { spyOn(inputRange, 'incrementValue'); - slider = inputRange.sliderMaxNode; event = { keyCode: 38, preventDefault: jasmine.createSpy('preventDefault'), @@ -369,13 +362,13 @@ describe('InputRange', () => { }); it('should increment value', () => { - inputRange.handleSliderKeyDown(event, slider); + inputRange.handleSliderKeyDown(event, 'max'); expect(inputRange.incrementValue).toHaveBeenCalledWith('max'); }); it('should call event.preventDefault()', () => { - inputRange.handleSliderKeyDown(event, slider); + inputRange.handleSliderKeyDown(event, 'max'); expect(event.preventDefault).toHaveBeenCalledWith(); }); @@ -383,7 +376,7 @@ describe('InputRange', () => { it('should not increment value if disabled', () => { inputRange = renderComponent(); spyOn(inputRange, 'incrementValue'); - inputRange.handleSliderKeyDown(event, slider); + inputRange.handleSliderKeyDown(event, 'max'); expect(inputRange.incrementValue).not.toHaveBeenCalled(); }); @@ -411,14 +404,14 @@ describe('InputRange', () => { }); it('should call event.preventDefault if not disabled', () => { - inputRange.handleTrackMouseDown(event, track, position); + inputRange.handleTrackMouseDown(event, position); expect(event.preventDefault).toHaveBeenCalledWith(); }); it('should not call event.preventDefault if disabled', () => { inputRange = renderComponent(); - inputRange.handleTrackMouseDown(event, track, position); + inputRange.handleTrackMouseDown(event, position); expect(event.preventDefault).not.toHaveBeenCalledWith(); }); @@ -426,7 +419,7 @@ describe('InputRange', () => { it('should not set a new position if disabled', () => { inputRange = renderComponent(); spyOn(inputRange, 'updatePosition'); - inputRange.handleTrackMouseDown(event, track, position); + inputRange.handleTrackMouseDown(event, position); expect(inputRange.updatePosition).not.toHaveBeenCalled(); }); @@ -435,7 +428,7 @@ describe('InputRange', () => { describe('if the new position is closer to `min` slider', () => { it('should set the position of the `min` slider', () => { position.x = 100; - inputRange.handleTrackMouseDown(event, track, position); + inputRange.handleTrackMouseDown(event, position); expect(inputRange.updatePosition).toHaveBeenCalledWith('min', position); }); @@ -444,7 +437,7 @@ describe('InputRange', () => { describe('if the new position is closer to `max` slider', () => { it('should set the position of the `max` slider', () => { position.x = 400; - inputRange.handleTrackMouseDown(event, track, position); + inputRange.handleTrackMouseDown(event, position); expect(inputRange.updatePosition).toHaveBeenCalledWith('max', position); }); @@ -461,7 +454,7 @@ describe('InputRange', () => { it('should set the position of the `max` slider', () => { position.x = 100; - inputRange.handleTrackMouseDown(event, track, position); + inputRange.handleTrackMouseDown(event, position); expect(inputRange.updatePosition).toHaveBeenCalledWith('max', position); }); @@ -496,7 +489,7 @@ describe('InputRange', () => { ); slider.dispatchEvent(mouseUpEvent); - expect(onChangeComplete).toHaveBeenCalledWith(inputRange, value); + expect(onChangeComplete).toHaveBeenCalledWith(value); }); it('should call onChangeComplete if value has changed since the start of interaction when only defaultValue was provided', () => { @@ -513,7 +506,7 @@ describe('InputRange', () => { ); slider.dispatchEvent(mouseUpEvent); - expect(onChangeComplete).toHaveBeenCalledWith(inputRange, value); + expect(onChangeComplete).toHaveBeenCalledWith(value); }); it('should not call onChangeComplete if value has not changed since the start of interaction', () => {