From 9ee2f8f70922ccdff21f6f138afb97d5ccd43fc2 Mon Sep 17 00:00:00 2001 From: SleepWalker Date: Tue, 26 Jul 2016 07:40:45 +0300 Subject: [PATCH] #120: Refactor popup --- src/components/ui/popup/PopupStack.jsx | 19 ++--- src/components/ui/popup/actions.js | 21 +++-- src/components/ui/popup/reducer.js | 10 +-- src/pages/profile/ProfilePage.jsx | 37 ++++---- tests/components/ui/popup/PopupStack.test.jsx | 84 ++++++++++++------- tests/components/ui/popup/reducer.test.js | 46 +++++----- 6 files changed, 121 insertions(+), 96 deletions(-) diff --git a/src/components/ui/popup/PopupStack.jsx b/src/components/ui/popup/PopupStack.jsx index 18982c9..f901322 100644 --- a/src/components/ui/popup/PopupStack.jsx +++ b/src/components/ui/popup/PopupStack.jsx @@ -30,18 +30,13 @@ export class PopupStack extends Component { transitionLeaveTimeout={500} > {popups.map((popup, index) => { - const Popup = popup.type; - - const defaultProps = { - onClose: this.onClose(popup) - }; - const props = typeof popup.props === 'function' - ? popup.props(defaultProps) - : {...defaultProps, ...popup.props}; + const {Popup} = popup; return ( -
- +
+
); })} @@ -53,9 +48,9 @@ export class PopupStack extends Component { return this.props.destroy.bind(null, popup); } - onOverlayClick(popup, popupProps) { + onOverlayClick(popup) { return (event) => { - if (event.target !== event.currentTarget || popupProps.disableOverlayClose) { + if (event.target !== event.currentTarget || popup.disableOverlayClose) { return; } diff --git a/src/components/ui/popup/actions.js b/src/components/ui/popup/actions.js index becf247..01e8a62 100644 --- a/src/components/ui/popup/actions.js +++ b/src/components/ui/popup/actions.js @@ -1,11 +1,22 @@ export const POPUP_CREATE = 'POPUP_CREATE'; -export function create(type, props = {}) { + +/** + * @param {object|ReactComponent} payload - react component or options + * @param {ReactComponent} payload.Popup + * @param {bool} [payload.disableOverlayClose=false] - do not allow hiding popup + * + * @return {object} + */ +export function create(payload) { + if (typeof payload === 'function') { + payload = { + Popup: payload + }; + } + return { type: POPUP_CREATE, - payload: { - type, - props - } + payload }; } diff --git a/src/components/ui/popup/reducer.js b/src/components/ui/popup/reducer.js index 9c34c6a..ed63bd8 100644 --- a/src/components/ui/popup/reducer.js +++ b/src/components/ui/popup/reducer.js @@ -6,20 +6,16 @@ export default combineReducers({ popups }); -function popups(popups = [], {type, payload}) { +function popups(popups = [], {type, payload = {}}) { switch (type) { case POPUP_CREATE: - if (!payload.type) { - throw new Error('Popup type is required'); + if (!payload.Popup) { + throw new Error('Popup is required'); } return popups.concat(payload); case POPUP_DESTROY: - if (!payload.type) { - throw new Error('Popup type is required'); - } - return popups.filter((popup) => popup !== payload); default: diff --git a/src/pages/profile/ProfilePage.jsx b/src/pages/profile/ProfilePage.jsx index f2b3817..846b5a1 100644 --- a/src/pages/profile/ProfilePage.jsx +++ b/src/pages/profile/ProfilePage.jsx @@ -74,24 +74,27 @@ export default connect(null, { function requestPassword(form) { return new Promise((resolve) => { - dispatch(createPopup(PasswordRequestForm, (props) => ({ - form, - disableOverlayClose: true, - onSubmit: () => { - form.beginLoading(); - sendData() - .catch((resp) => { - if (resp.errors) { - form.setErrors(resp.errors); - } + dispatch(createPopup({ + Popup(props) { + const onSubmit = () => { + form.beginLoading(); + sendData() + .catch((resp) => { + if (resp.errors) { + form.setErrors(resp.errors); + } - return Promise.reject(resp); - }) - .then(resolve) - .then(props.onClose) - .finally(() => form.endLoading()); - } - }))); + return Promise.reject(resp); + }) + .then(resolve) + .then(props.onClose) + .finally(() => form.endLoading()); + }; + + return ; + }, + disableOverlayClose: true + })); }); } } diff --git a/tests/components/ui/popup/PopupStack.test.jsx b/tests/components/ui/popup/PopupStack.test.jsx index 70182d1..8e42f78 100644 --- a/tests/components/ui/popup/PopupStack.test.jsx +++ b/tests/components/ui/popup/PopupStack.test.jsx @@ -1,10 +1,11 @@ import React from 'react'; -import { shallow } from 'enzyme'; +import { shallow, mount } from 'enzyme'; import { PopupStack } from 'components/ui/popup/PopupStack'; +import styles from 'components/ui/popup/popup.scss'; -function DummyPopup() {} +function DummyPopup() {return null;} describe('', () => { it('renders all popup components', () => { @@ -12,10 +13,10 @@ describe('', () => { destroy: () => {}, popups: [ { - type: DummyPopup + Popup: DummyPopup }, { - type: DummyPopup + Popup: DummyPopup } ] }; @@ -24,7 +25,7 @@ describe('', () => { expect(component.find(DummyPopup)).to.have.length(2); }); - it('should pass provided props', () => { + it('should pass onClose as props', () => { const expectedProps = { foo: 'bar' }; @@ -33,47 +34,29 @@ describe('', () => { destroy: () => {}, popups: [ { - type: DummyPopup, - props: expectedProps - } - ] - }; - const component = shallow(); + Popup: (props = {}) => { + expect(props.onClose).to.be.a('function'); - expect(component.find(DummyPopup).prop('foo')).to.be.equal(expectedProps.foo); - }); - - it('should use props as proxy if it is function', () => { - const expectedProps = { - foo: 'bar' - }; - - const props = { - destroy: () => {}, - popups: [ - { - type: DummyPopup, - props: (props) => { - expect(props).to.have.property('onClose'); - - return expectedProps; + return ; } } ] }; - const component = shallow(); + const component = mount(); - expect(component.find(DummyPopup).props()).to.be.deep.equal(expectedProps); + const popup = component.find(DummyPopup); + expect(popup).to.have.length(1); + expect(popup.props()).to.deep.equal(expectedProps); }); it('should hide popup, when onClose called', () => { const props = { popups: [ { - type: DummyPopup + Popup: DummyPopup }, { - type: DummyPopup + Popup: DummyPopup } ], destroy: sinon.stub() @@ -85,4 +68,41 @@ describe('', () => { sinon.assert.calledOnce(props.destroy); sinon.assert.calledWith(props.destroy, sinon.match.same(props.popups[1])); }); + + it('should hide popup, when overlay clicked', () => { + const preventDefault = sinon.stub(); + const props = { + destroy: sinon.stub(), + popups: [ + { + Popup: DummyPopup + } + ] + }; + const component = shallow(); + + const overlay = component.find(`.${styles.overlay}`); + overlay.simulate('click', {target: 1, currentTarget: 1, preventDefault}); + + sinon.assert.calledOnce(props.destroy); + sinon.assert.calledOnce(preventDefault); + }); + + it('should hide popup on overlay click if disableOverlayClose', () => { + const props = { + destroy: sinon.stub(), + popups: [ + { + Popup: DummyPopup, + disableOverlayClose: true + } + ] + }; + const component = shallow(); + + const overlay = component.find(`.${styles.overlay}`); + overlay.simulate('click', {target: 1, currentTarget: 1, preventDefault() {}}); + + sinon.assert.notCalled(props.destroy); + }); }); diff --git a/tests/components/ui/popup/reducer.test.js b/tests/components/ui/popup/reducer.test.js index 7ae2d68..98ca28d 100644 --- a/tests/components/ui/popup/reducer.test.js +++ b/tests/components/ui/popup/reducer.test.js @@ -11,36 +11,38 @@ describe('popup/reducer', () => { describe('#create', () => { it('should create popup', () => { + const actual = reducer(undefined, create({ + Popup: FakeComponent + })); + + expect(actual.popups[0]).to.be.deep.equal({ + Popup: FakeComponent + }); + }); + + it('should support shortcut popup creation', () => { const actual = reducer(undefined, create(FakeComponent)); expect(actual.popups[0]).to.be.deep.equal({ - type: FakeComponent, - props: {} + Popup: FakeComponent }); }); - it('should store props', () => { - const expectedProps = {foo: 'bar'}; - const actual = reducer(undefined, create(FakeComponent, expectedProps)); - - expect(actual.popups[0]).to.be.deep.equal({ - type: FakeComponent, - props: expectedProps - }); - }); - - it('should not remove existed popups', () => { - let actual = reducer(undefined, create(FakeComponent)); - actual = reducer(actual, create(FakeComponent)); + it('should create multiple popups', () => { + let actual = reducer(undefined, create({ + Popup: FakeComponent + })); + actual = reducer(actual, create({ + Popup: FakeComponent + })); expect(actual.popups[1]).to.be.deep.equal({ - type: FakeComponent, - props: {} + Popup: FakeComponent }); }); it('throws when no type provided', () => { - expect(() => reducer(undefined, create())).to.throw('Popup type is required'); + expect(() => reducer(undefined, create())).to.throw('Popup is required'); }); }); @@ -62,17 +64,15 @@ describe('popup/reducer', () => { }); it('should not remove something, that it should not', () => { - state = reducer(state, create('foo')); + state = reducer(state, create({ + Popup: FakeComponent + })); state = reducer(state, destroy(popup)); expect(state.popups).to.have.length(1); expect(state.popups[0]).to.not.equal(popup); }); - - it('throws when no type provided', () => { - expect(() => reducer(undefined, destroy({}))).to.throw('Popup type is required'); - }); }); });