diff --git a/src/__tests__/index.js b/src/__tests__/index.js index ce7dbdd..d934805 100644 --- a/src/__tests__/index.js +++ b/src/__tests__/index.js @@ -155,29 +155,28 @@ test('children can be an array (for preact support)', () => { test('onToggle gets called in controlled prop scenario', () => { const spy = jest.fn() - const {wrapper} = setup({on: false, onToggle: spy}) + const {setOn, setOff, wrapper} = setup({on: false, onToggle: spy}) + setOff() expect(spy).not.toHaveBeenCalled() + setOn() + expect(spy).toHaveBeenLastCalledWith(true, expect.anything()) wrapper.setProps({on: true}) - expect(spy).toHaveBeenCalled() expect(spy.mock.calls.length).toBe(1) }) -test('onToggle gets called with fresh state in controlled prop scenario', () => { +test("onToggle doesn't get called in controlled prop scenario - on external prop change", () => { const spy = jest.fn() const {wrapper} = setup({on: false, onToggle: spy}) + expect(spy).not.toHaveBeenCalled() wrapper.setProps({on: true}) - expect(spy).toHaveBeenLastCalledWith(true, expect.anything()) - expect(spy.mock.calls.length).toBe(1) + expect(spy).not.toHaveBeenCalled() }) -test('onToggle gets called on internal state change in controlled prop scenario', () => { +test('onToggle gets called with fresh state in controlled prop scenario', () => { const spy = jest.fn() - const {setOn, setOff, wrapper} = setup({on: false, onToggle: spy}) - setOff() - expect(spy).not.toHaveBeenCalled() + const {setOn} = setup({on: false, onToggle: spy}) setOn() expect(spy).toHaveBeenLastCalledWith(true, expect.anything()) - wrapper.setProps({on: true}) expect(spy.mock.calls.length).toBe(1) }) diff --git a/src/index.js b/src/index.js index bd71e47..2cbee38 100644 --- a/src/index.js +++ b/src/index.js @@ -57,9 +57,11 @@ class Toggle extends Component { }), }) - getTogglerStateAndHelpers() { + // having an override here makes getTogglerProps return "outdated" aria-expanded for that case + // is that a problem? + getTogglerStateAndHelpers(on = this.getOn()) { return { - on: this.getOn(), + on, getTogglerProps: this.getTogglerProps, getInputTogglerProps: this.getInputTogglerProps, getElementTogglerProps: this.getElementTogglerProps, @@ -70,25 +72,24 @@ class Toggle extends Component { } setOnState = (state = !this.getOn()) => { - const cb = - this.getOn() === state - ? noop - : () => { - this.props.onToggle(state, this.getTogglerStateAndHelpers()) - } - this.setState({on: state}, cb) + if (this.getOn() === state) { + return + } + + // should we differentiate timing for uncontrolled/controlled modes? + this.props.onToggle(state, this.getTogglerStateAndHelpers(state)) + + if (this.isOnControlled()) { + return + } + + this.setState({on: state}) } setOn = this.setOnState.bind(this, true) setOff = this.setOnState.bind(this, false) toggle = this.setOnState.bind(this, undefined) - componentWillReceiveProps({on}) { - if (on !== this.props.on && on !== this.state.on) { - this.setOnState(on) - } - } - render() { const renderProp = unwrapArray(this.props.children) return renderProp(this.getTogglerStateAndHelpers())