From 301c8aa18020627cef665d521fe16780b0bfb632 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoffer=20=C3=85str=C3=B6m?= Date: Wed, 15 Jan 2020 12:04:30 +0100 Subject: [PATCH] refactor: object selections (#256) --- apis/nucleus/src/components/Cell.jsx | 21 +- .../src/components/listbox/ListBoxPopover.jsx | 16 +- apis/nucleus/src/hooks/useAppSelections.js | 31 +-- .../useObjectSelections.js} | 63 +++-- .../__tests__/object-selections.spec.js | 257 ------------------ apis/nucleus/src/selections/index.js | 4 - apis/nucleus/src/stores/selectionsStore.js | 4 +- 7 files changed, 79 insertions(+), 317 deletions(-) rename apis/nucleus/src/{selections/object-selections.js => hooks/useObjectSelections.js} (62%) delete mode 100644 apis/nucleus/src/selections/__tests__/object-selections.spec.js delete mode 100644 apis/nucleus/src/selections/index.js diff --git a/apis/nucleus/src/components/Cell.jsx b/apis/nucleus/src/components/Cell.jsx index 9cf6bfdb4..74ed1188f 100644 --- a/apis/nucleus/src/components/Cell.jsx +++ b/apis/nucleus/src/components/Cell.jsx @@ -14,7 +14,7 @@ import Supernova from './Supernova'; import useRect from '../hooks/useRect'; import useLayout from '../hooks/useLayout'; import InstanceContext from '../contexts/InstanceContext'; -import { createObjectSelectionAPI } from '../selections'; +import useObjectSelections from '../hooks/useObjectSelections'; const initialState = err => ({ loading: false, @@ -90,7 +90,6 @@ const handleModal = ({ sn, layout, model }) => { return; } if (selections.id === model.id) { - selections.setLayout(layout); if (layout && layout.qSelectionInfo && layout.qSelectionInfo.qInSelections && !selections.isModal()) { const { targets } = sn.generator.qae.data; const firstPropertyPath = targets[0].propertyPath; @@ -139,14 +138,14 @@ const getType = async ({ types, name, version }) => { return SN; }; -const loadType = async ({ dispatch, types, name, version, layout, model, app }) => { +const loadType = async ({ dispatch, types, name, version, layout, model, app, selections }) => { try { const snType = await getType({ types, name, version }); // Layout might have changed since we requested the new type -> quick return if (layout.visualization !== name) { return undefined; } - const selections = createObjectSelectionAPI(model, app); + const sn = snType.create({ model, app, @@ -169,12 +168,13 @@ const Cell = forwardRef(({ corona, model, initialSnContext, initialSnOptions, in const { translator, language } = useContext(InstanceContext); const theme = useTheme(); + const cellRef = useRef(); const [state, dispatch] = useReducer(contentReducer, initialState(initialError)); const [layout, { validating, canCancel, canRetry }, longrunning] = useLayout(model); const [contentRef, contentRect, , contentNode] = useRect(); const [snContext, setSnContext] = useState(initialSnContext); const [snOptions, setSnOptions] = useState(initialSnOptions); - const cellRef = useRef(); + const [selections] = useObjectSelections(app, model); useEffect(() => { if (initialError) { @@ -190,7 +190,16 @@ const Cell = forwardRef(({ corona, model, initialSnContext, initialSnOptions, in handleModal({ sn: state.sn, layout, model }); }; const load = async (withLayout, version) => { - const sn = await loadType({ dispatch, types, name: withLayout.visualization, version, layout, model, app }); + const sn = await loadType({ + dispatch, + types, + name: withLayout.visualization, + version, + layout, + model, + app, + selections, + }); if (sn) { dispatch({ type: 'LOADED', sn }); onMount(); diff --git a/apis/nucleus/src/components/listbox/ListBoxPopover.jsx b/apis/nucleus/src/components/listbox/ListBoxPopover.jsx index 3f43bd343..94966ad14 100644 --- a/apis/nucleus/src/components/listbox/ListBoxPopover.jsx +++ b/apis/nucleus/src/components/listbox/ListBoxPopover.jsx @@ -13,12 +13,12 @@ import useLayout from '../../hooks/useLayout'; import ListBox from './ListBox'; import createListboxSelectionToolbar from './listbox-selection-toolbar'; -import { createObjectSelectionAPI } from '../../selections'; import SelectionToolbarWithDefault, { SelectionToolbar } from '../SelectionToolbar'; import InstanceContext from '../../contexts/InstanceContext'; import ListBoxSearch from './ListBoxSearch'; +import useObjectSelections from '../../hooks/useObjectSelections'; export default function ListBoxPopover({ alignTo, show, close, app, fieldName, stateName = '$' }) { const theme = useTheme(); @@ -70,23 +70,21 @@ export default function ListBoxPopover({ alignTo, show, close, app, fieldName, s const moreAlignTo = useRef(); const [showSelectionsMenu, setShowSelectionsMenu] = useState(false); + const [selections] = useObjectSelections(app, model); - if (!model || !layout || !translator) { + if (!model || !layout || !translator || !selections) { return null; } - const sel = createObjectSelectionAPI(model, app); - sel.setLayout(layout); - const isLocked = layout.qListObject.qDimensionInfo.qLocked === true; const open = show && Boolean(alignTo.current); if (open) { - sel.goModal('/qListObjectDef'); + selections.goModal('/qListObjectDef'); } const popoverClose = (e, reason) => { const accept = reason !== 'escapeKeyDown'; - sel.noModal(accept); + selections.noModal(accept); close(); }; @@ -144,7 +142,7 @@ export default function ListBoxPopover({ alignTo, show, close, app, fieldName, s popoverClose(null, 'escapeKeyDown')} @@ -154,7 +152,7 @@ export default function ListBoxPopover({ alignTo, show, close, app, fieldName, s
- + {showSelectionsMenu && ( { - if (err.code === 6003) { - // If another object already is in modal -> abort and take over - return appSelections.abortModal().then(() => object.beginSelections(Array.isArray(path) ? path : [path])); - } - throw err; - }); + object.beginSelections(Array.isArray(path) ? path : [path]).catch(err => { + if (err.code === 6003) { + // If another object already is in modal -> abort and take over + return appSelections.abortModal().then(() => object.beginSelections(Array.isArray(path) ? path : [path])); + } + throw err; + }); return Promise.resolve(); } modalObjectStore.set(key, null); @@ -40,9 +39,9 @@ function createAppSelections({ app, currentSelectionsLayout, navState }) { isInModal() { return modalObjectStore.get(key) !== null; }, - isModal(objectModel) { + isModal(object) { // TODO check model state - return objectModel ? modalObjectStore.get(key) === objectModel : modalObjectStore.get(key) !== null; + return object ? modalObjectStore.get(key) === object : modalObjectStore.get(key) !== null; }, abortModal(accept = true) { if (!modalObjectStore.get(key)) { diff --git a/apis/nucleus/src/selections/object-selections.js b/apis/nucleus/src/hooks/useObjectSelections.js similarity index 62% rename from apis/nucleus/src/selections/object-selections.js rename to apis/nucleus/src/hooks/useObjectSelections.js index 8b63b6955..4740791f2 100644 --- a/apis/nucleus/src/selections/object-selections.js +++ b/apis/nucleus/src/hooks/useObjectSelections.js @@ -1,7 +1,9 @@ /* eslint no-underscore-dangle: 0 */ -import eventmixin from './event-mixin'; - -import { appSelectionsStore } from '../stores/selectionsStore'; +import { useEffect } from 'react'; +import { useObjectSelectionsStore } from '../stores/selectionsStore'; +import useAppSelections from './useAppSelections'; +import eventmixin from '../selections/event-mixin'; +import useLayout from './useLayout'; const event = () => { let prevented = false; @@ -13,15 +15,10 @@ const event = () => { }; }; -export default function(model, app) { - if (model._selections) { - return model._selections; - } - // const appAPI = () => app._selections; - const appAPI = () => appSelectionsStore.get(app.id); - let hasSelected = false; +function createObjectSelections({ appSelections, model }) { + let layout; let isActive = false; - let layout = {}; + let hasSelected = false; /** * @interface @@ -45,7 +42,7 @@ export default function(model, app) { } isActive = true; this.emit('activated'); - return appAPI().switchModal(model, paths, true); + return appSelections.switchModal(model, paths, true); }, /** * @returns {Promise} @@ -66,7 +63,7 @@ export default function(model, app) { isActive = false; this.emit('confirmed'); this.emit('deactivated'); - return appAPI().switchModal(null, null, true); + return appSelections.switchModal(null, null, true); }, /** * @returns {Promise} @@ -76,7 +73,7 @@ export default function(model, app) { isActive = false; this.emit('canceled'); // FIXME - spelling? this.emit('deactivated'); - return appAPI().switchModal(null, null, false, false); + return appSelections.switchModal(null, null, false, false); }, /** * @param {object} s @@ -85,7 +82,7 @@ export default function(model, app) { */ select(s) { const b = this.begin([s.params[0]]); - if (!appAPI().isModal()) { + if (!appSelections.isModal()) { return Promise.resolve(); } hasSelected = true; @@ -104,7 +101,7 @@ export default function(model, app) { if (layout && layout.qListObject && layout.qListObject.qDimensionInfo) { return !layout.qListObject.qDimensionInfo.qLocked; } - return hasSelected && layout.qSelectionInfo.qMadeSelections; + return hasSelected || (layout && layout.qSelectionInfo.qMadeSelections); }, /** * @returns {boolean} @@ -113,7 +110,7 @@ export default function(model, app) { if (layout && layout.qListObject && layout.qListObject.qDimensionInfo) { return !layout.qListObject.qDimensionInfo.qLocked; } - return hasSelected && layout.qSelectionInfo.qMadeSelections; + return hasSelected || (layout && layout.qSelectionInfo.qMadeSelections); }, /** * @returns {boolean} @@ -131,26 +128,46 @@ export default function(model, app) { /** * @returns {boolean} */ - isModal: () => appAPI().isModal(model), + isModal: () => appSelections.isModal(model), /** * @param {string[]} paths * @returns {Promise} */ - goModal: paths => appAPI().switchModal(model, paths, false), + goModal: paths => appSelections.switchModal(model, paths, false), /** * @param {boolean} [accept=false] * @returns {Promise} */ - noModal: (accept = false) => appAPI().switchModal(null, null, accept), + noModal: (accept = false) => appSelections.switchModal(null, null, accept), /** * @returns {Promise} */ - abortModal: () => appAPI().abortModal(true), + abortModal: () => appSelections.abortModal(true), }; eventmixin(api); - model._selections = api; // eslint-disable-line no-param-reassign - return api; } + +export default function useObjectSelections(app, model) { + const [appSelections] = useAppSelections(app); + const [layout] = useLayout(model); + const key = model ? model.id : null; + const [objectSelectionsStore] = useObjectSelectionsStore(); + let objectSelections = objectSelectionsStore.get(key); + + useEffect(() => { + if (!appSelections || !model || objectSelections) return; + + objectSelections = createObjectSelections({ appSelections, model }); + objectSelectionsStore.set(key, objectSelections); + }, [appSelections, model]); + + useEffect(() => { + if (!objectSelections) return; + objectSelections.setLayout(layout); + }, [objectSelections, layout]); + + return [objectSelections]; +} diff --git a/apis/nucleus/src/selections/__tests__/object-selections.spec.js b/apis/nucleus/src/selections/__tests__/object-selections.spec.js deleted file mode 100644 index 39837fbb3..000000000 --- a/apis/nucleus/src/selections/__tests__/object-selections.spec.js +++ /dev/null @@ -1,257 +0,0 @@ -/* eslint no-param-reassign:0 no-underscore-dangle:0 */ -import { appSelectionsStore } from '../../stores/selectionsStore'; - -describe('object-selections', () => { - let create; - let sandbox; - let ex; - let app; - let selections; - let model; - - const mixin = api => { - api.emit = sandbox.stub(); - }; - - before(() => { - sandbox = sinon.createSandbox(); - ex = sandbox.stub(); - [{ default: create }] = aw.mock([['**/event-mixin.js', () => ex]], ['../object-selections.js']); - }); - afterEach(() => { - sandbox.reset(); - }); - beforeEach(() => { - app = { - id: 'my-app', - }; - selections = { - switchModal: sandbox.stub(), - isModal: sandbox.stub(), - abortModal: sandbox.stub(), - }; - appSelectionsStore.set(app.id, selections); - model = { - clearSelections: sandbox.stub(), - resetMadeSelections: sandbox.stub(), - }; - }); - - it('should initiate with mixin', () => { - const c = create(model, app); - expect(ex).to.have.been.calledWithExactly(c); - }); - - it('should cache the instance', () => { - const c = create(model, app); - expect(model._selections).to.equal(c); - expect(ex.callCount).to.equal(1); - - const c2 = create(model, app); - expect(c2).to.equal(c); - }); - - it('begin() should emit events and swith modal', () => { - const c = create(model, app); - mixin(c); - selections.switchModal.returns('switch'); - - expect(c.isActive()).to.equal(false); - expect(c.begin('paths')).to.equal('switch'); - expect(c.emit.firstCall).to.have.been.calledWith('activate'); - expect(c.emit.secondCall).to.have.been.calledWithExactly('activated'); - expect(selections.switchModal).to.have.been.calledWithExactly(model, 'paths', true); - expect(c.isActive()).to.equal(true); - }); - - it('clear() should emit events and reset made selections', () => { - const c = create(model, app); - mixin(c); - selections.switchModal.returns('switch'); - model.clearSelections.returns('clear'); - model.resetMadeSelections.returns('reset'); - - expect(c.clear()).to.equal('reset'); - expect(c.emit.firstCall).to.have.been.calledWithExactly('cleared'); - expect(model.resetMadeSelections).to.have.been.calledWithExactly(); - }); - - it('clear() should emit events and clear selections', () => { - const c = create(model, app); - mixin(c); - selections.switchModal.returns('switch'); - model.clearSelections.returns('clear'); - c.setLayout({ qListObject: {} }); - - expect(c.clear()).to.equal('clear'); - expect(c.emit.firstCall).to.have.been.calledWithExactly('cleared'); - expect(model.clearSelections).to.have.been.calledWithExactly('/qListObjectDef'); - }); - - it('confirm() should emit events and switch modal', () => { - const c = create(model, app); - mixin(c); - selections.switchModal.returns('switch'); - - expect(c.confirm()).to.equal('switch'); - expect(c.emit.firstCall).to.have.been.calledWithExactly('confirmed'); - expect(c.emit.secondCall).to.have.been.calledWithExactly('deactivated'); - expect(selections.switchModal).to.have.been.calledWithExactly(null, null, true); - }); - - it('cancel() should emit events and switch modal', () => { - const c = create(model, app); - mixin(c); - selections.switchModal.returns('switch'); - - expect(c.cancel()).to.equal('switch'); - expect(c.emit.firstCall).to.have.been.calledWithExactly('canceled'); - expect(c.emit.secondCall).to.have.been.calledWithExactly('deactivated'); - expect(selections.switchModal).to.have.been.calledWithExactly(null, null, false, false); - }); - - it('select() should return early if app modality is false', () => { - const c = create(model, app); - mixin(c); - selections.isModal.returns(false); - const begin = sandbox.stub(c, 'begin'); - const b = { then: sandbox.stub() }; - begin.returns(b); - - const s = { params: ['path'] }; - - c.select(s); - - expect(begin).to.have.been.calledWithExactly(['path']); - expect(b.then.callCount).to.equal(0); - }); - - it('select() should apply model selections', async () => { - const c = create(model, app); - mixin(c); - model.myMethod = sandbox.stub(); - selections.isModal.returns(true); - const begin = sandbox.stub(c, 'begin'); - const b = Promise.resolve(); - begin.returns(b); - model.myMethod.returns(Promise.resolve()); - - const s = { method: 'myMethod', params: ['path', 'p'] }; - - await c.select(s); - - expect(begin).to.have.been.calledWithExactly(['path']); - expect(model.myMethod).to.have.been.calledWithExactly('path', 'p'); - }); - - it('select() should clear if selection was not successful', async () => { - const c = create(model, app); - mixin(c); - model.myMethod = sandbox.stub(); - selections.isModal.returns(true); - const begin = sandbox.stub(c, 'begin'); - const clear = sandbox.stub(c, 'clear'); - const b = Promise.resolve(); - begin.returns(b); - - model.myMethod.returns(Promise.resolve(false)); - - const s = { method: 'myMethod', params: ['path', 'p'] }; - - await c.select(s); - - expect(begin).to.have.been.calledWithExactly(['path']); - expect(clear).to.have.been.calledWithExactly(); - }); - - it('canClear should return true when listobject is not locked', () => { - const c = create(model, app); - c.setLayout({ qListObject: { qDimensionInfo: { qLocked: true } } }); - expect(c.canClear()).to.equal(false); - - c.setLayout({ qListObject: { qDimensionInfo: { qLocked: false } } }); - expect(c.canClear()).to.equal(true); - }); - - it('canClear should return true when selections have been made', () => { - const c = create(model, app); - mixin(c); - const begin = sandbox.stub(c, 'begin'); - begin.returns({ then: () => {} }); - selections.isModal.returns(true); - - c.setLayout({ qSelectionInfo: { qMadeSelections: true } }); - expect(c.canClear()).to.equal(false); - - c.select({ params: [] }); - expect(c.canClear()).to.equal(true); - }); - - it('canConfirm should return true when listobject is not locked', () => { - const c = create(model, app); - c.setLayout({ qListObject: { qDimensionInfo: { qLocked: true } } }); - expect(c.canConfirm()).to.equal(false); - - c.setLayout({ qListObject: { qDimensionInfo: { qLocked: false } } }); - expect(c.canConfirm()).to.equal(true); - }); - - it('canConfirm should return true when selections have been made', () => { - const c = create(model, app); - mixin(c); - const begin = sandbox.stub(c, 'begin'); - begin.returns({ then: () => {} }); - selections.isModal.returns(true); - - c.setLayout({ qSelectionInfo: { qMadeSelections: true } }); - expect(c.canConfirm()).to.equal(false); - - c.select({ params: [] }); - expect(c.canConfirm()).to.equal(true); - }); - - it('canCancel should return inverse of locked when listobject', () => { - const c = create(model, app); - c.setLayout({ qListObject: { qDimensionInfo: { qLocked: true } } }); - expect(c.canCancel()).to.equal(false); - - c.setLayout({ qListObject: { qDimensionInfo: { qLocked: false } } }); - expect(c.canCancel()).to.equal(true); - }); - - it('canCancel should return true when not listobject', () => { - const c = create(model, app); - c.setLayout({}); - expect(c.canCancel()).to.equal(true); - }); - - it('isModal should return app isModal', () => { - const c = create(model, app); - selections.isModal.withArgs(model).returns('maybe'); - expect(c.isModal()).to.equal('maybe'); - }); - - it('goModal should switch app modality', () => { - const c = create(model, app); - selections.switchModal.returns('switch'); - expect(c.goModal('paths')).to.equal('switch'); - expect(selections.switchModal).to.have.been.calledWithExactly(model, 'paths', false); - }); - - it('noModal should switch app modality', () => { - const c = create(model, app); - selections.switchModal.returns('switch'); - expect(c.noModal()).to.equal('switch'); - expect(selections.switchModal).to.have.been.calledWithExactly(null, null, false); - - c.noModal(true); - expect(selections.switchModal.secondCall).to.have.been.calledWithExactly(null, null, true); - }); - - it('abortModal should call app abortModal', () => { - const c = create(model, app); - selections.abortModal.returns('ab'); - expect(c.abortModal()).to.equal('ab'); - expect(selections.abortModal).to.have.been.calledWithExactly(true); - }); -}); diff --git a/apis/nucleus/src/selections/index.js b/apis/nucleus/src/selections/index.js deleted file mode 100644 index 70ff2d3f3..000000000 --- a/apis/nucleus/src/selections/index.js +++ /dev/null @@ -1,4 +0,0 @@ -import createObjectSelectionAPI from './object-selections'; - -// eslint-disable-next-line import/prefer-default-export -export { createObjectSelectionAPI }; diff --git a/apis/nucleus/src/stores/selectionsStore.js b/apis/nucleus/src/stores/selectionsStore.js index e0d329f5b..9ddaaa377 100644 --- a/apis/nucleus/src/stores/selectionsStore.js +++ b/apis/nucleus/src/stores/selectionsStore.js @@ -1,6 +1,6 @@ import createKeyStore from './createKeyStore'; const [useAppSelectionsStore, appSelectionsStore] = createKeyStore({}); -const [useObjectSelectionsStore] = createKeyStore({}); +const [useObjectSelectionsStore, objectSelectionsStore] = createKeyStore({}); -export { useAppSelectionsStore, useObjectSelectionsStore, appSelectionsStore }; +export { useAppSelectionsStore, useObjectSelectionsStore, appSelectionsStore, objectSelectionsStore };