From 7e6ea40ab18c99b2018233628cedc63d74f5963d Mon Sep 17 00:00:00 2001
From: Jack Reed <phillipjreed@gmail.com>
Date: Fri, 16 Oct 2020 07:35:50 -0600
Subject: [PATCH] Remove workspace based collectiondialog
---
.../src/components/CollectionDialog.test.js | 7 +--
.../src/components/ManifestListItem.test.js | 7 ---
__tests__/src/reducers/windows.test.js | 12 ++---
__tests__/src/sagas/windows.test.js | 4 +-
src/components/CollectionDialog.js | 45 +++++++++----------
src/components/ManifestListItem.js | 11 +----
src/components/PrimaryWindow.js | 2 +-
src/components/SelectCollection.js | 6 +--
src/components/WorkspaceArea.js | 5 ---
src/containers/CollectionDialog.js | 10 ++---
src/containers/ManifestListItem.js | 1 -
src/containers/SelectCollection.js | 2 +-
src/containers/WorkspaceArea.js | 1 -
src/state/actions/action-types.js | 2 -
src/state/actions/window.js | 8 ++--
src/state/actions/workspace.js | 17 -------
src/state/reducers/windows.js | 4 +-
src/state/reducers/workspace.js | 10 -----
src/state/sagas/app.js | 1 -
src/state/sagas/windows.js | 4 +-
20 files changed, 49 insertions(+), 110 deletions(-)
diff --git a/__tests__/src/components/CollectionDialog.test.js b/__tests__/src/components/CollectionDialog.test.js
index d821f7dfc..5c0a08380 100644
--- a/__tests__/src/components/CollectionDialog.test.js
+++ b/__tests__/src/components/CollectionDialog.test.js
@@ -12,6 +12,7 @@ import collection from '../../fixtures/version-2/collection.json';
/** */
function createWrapper(props) {
const manifest = Utils.parseManifest(props.manifest ? props.manifest : collection);
+ CollectionDialog.prototype.dialogContainer = () => true;
return shallow(
<CollectionDialog
addWindow={() => {}}
@@ -41,10 +42,4 @@ describe('CollectionDialog', () => {
expect(wrapper.find(DialogActions).find(Button).first().simulate('click'));
expect(hideCollectionDialog).toHaveBeenCalled();
});
- it('clicking the hide button fires hideWindowCollectionDialog in window variant', () => {
- const hideWindowCollectionDialog = jest.fn();
- const wrapper = createWrapper({ hideWindowCollectionDialog, variant: 'window' });
- expect(wrapper.find(DialogActions).find(Button).first().simulate('click'));
- expect(hideWindowCollectionDialog).toHaveBeenCalled();
- });
});
diff --git a/__tests__/src/components/ManifestListItem.test.js b/__tests__/src/components/ManifestListItem.test.js
index 72f183c72..5a6620f1b 100644
--- a/__tests__/src/components/ManifestListItem.test.js
+++ b/__tests__/src/components/ManifestListItem.test.js
@@ -68,13 +68,6 @@ describe('ManifestListItem', () => {
expect(wrapper.find('.mirador-manifest-list-item-provider').children().text()).toEqual('addedFromUrl');
});
- it('when clicking a collection fires the showCollectionDialog', () => {
- const showCollectionDialog = jest.fn();
- const wrapper = createWrapper({ isCollection: true, showCollectionDialog });
- wrapper.find(ButtonBase).simulate('click');
- expect(showCollectionDialog).toHaveBeenCalledTimes(1);
- });
-
it('displays a collection label for collections', () => {
const wrapper = createWrapper({ isCollection: true });
expect(wrapper.text()).toContain('collectionxyz');
diff --git a/__tests__/src/reducers/windows.test.js b/__tests__/src/reducers/windows.test.js
index 467243be3..be79e54d6 100644
--- a/__tests__/src/reducers/windows.test.js
+++ b/__tests__/src/reducers/windows.test.js
@@ -382,11 +382,11 @@ describe('windows reducer', () => {
})).toEqual({ new: 'stuff' });
});
- describe('SHOW_WINDOW_COLLECTION_DIALOG', () => {
- it('handles SHOW_WINDOW_COLLECTION_DIALOG by toggling the given window\'s collection dialog', () => {
+ describe('SHOW_COLLECTION_DIALOG', () => {
+ it('handles SHOW_COLLECTION_DIALOG by toggling the given window\'s collection dialog', () => {
const beforeState = { abc123: { collectionDialogOn: false } };
const action = {
- collectionPath: [], manifestId: 'def456', type: ActionTypes.SHOW_WINDOW_COLLECTION_DIALOG, windowId: 'abc123',
+ collectionPath: [], manifestId: 'def456', type: ActionTypes.SHOW_COLLECTION_DIALOG, windowId: 'abc123',
};
const expectedState = {
abc123: { collectionDialogOn: true, collectionManifestId: 'def456', collectionPath: [] },
@@ -396,15 +396,15 @@ describe('windows reducer', () => {
});
});
- describe('HIDE_WINDOW_COLLECTION_DIALOG', () => {
- it('handles HIDE_WINDOW_COLLECTION_DIALOG by toggling the given window\'s collection dialog', () => {
+ describe('HIDE_COLLECTION_DIALOG', () => {
+ it('handles HIDE_COLLECTION_DIALOG by toggling the given window\'s collection dialog', () => {
const beforeState = {
abc123: {
collectionDialogOn: true, collectionManifestId: 'def456', collectionPath: [],
},
};
const action = {
- type: ActionTypes.HIDE_WINDOW_COLLECTION_DIALOG,
+ type: ActionTypes.HIDE_COLLECTION_DIALOG,
windowId: 'abc123',
};
diff --git a/__tests__/src/sagas/windows.test.js b/__tests__/src/sagas/windows.test.js
index 81223e470..034e08edf 100644
--- a/__tests__/src/sagas/windows.test.js
+++ b/__tests__/src/sagas/windows.test.js
@@ -453,7 +453,7 @@ describe('window-level sagas', () => {
action: {
collectionPath: [],
manifestId: 'manifest.json',
- type: 'mirador/SHOW_WINDOW_COLLECTION_DIALOG',
+ type: 'mirador/SHOW_COLLECTION_DIALOG',
windowId: 'x',
},
})
@@ -471,7 +471,7 @@ describe('window-level sagas', () => {
action: {
collectionPath: [],
manifestId: 'manifest.json',
- type: 'mirador/SHOW_WINDOW_COLLECTION_DIALOG',
+ type: 'mirador/SHOW_COLLECTION_DIALOG',
windowId: 'x',
},
})
diff --git a/src/components/CollectionDialog.js b/src/components/CollectionDialog.js
index a1412e9bb..e1aba1a6e 100644
--- a/src/components/CollectionDialog.js
+++ b/src/components/CollectionDialog.js
@@ -57,19 +57,10 @@ export class CollectionDialog extends Component {
/** */
hideDialog() {
const {
- hideCollectionDialog, hideWindowCollectionDialog, variant, windowId,
+ hideCollectionDialog, windowId,
} = this.props;
- if (variant === 'window') {
- hideWindowCollectionDialog(windowId);
- } else {
- hideCollectionDialog();
- }
- }
- /** */
- showCollectionDialog(...args) {
- const { showCollectionDialog, showWindowCollectionDialog, variant } = this.props;
- return variant === 'window' ? showWindowCollectionDialog(...args) : showCollectionDialog(...args);
+ hideCollectionDialog(windowId);
}
/** */
@@ -77,17 +68,18 @@ export class CollectionDialog extends Component {
const {
collectionPath,
manifestId,
+ showCollectionDialog,
windowId,
} = this.props;
- this.showCollectionDialog(c.id, [...collectionPath, manifestId], windowId);
+ showCollectionDialog(c.id, [...collectionPath, manifestId], windowId);
}
/** */
goToPreviousCollection() {
- const { collectionPath, windowId } = this.props;
+ const { collectionPath, showCollectionDialog, windowId } = this.props;
- this.showCollectionDialog(
+ showCollectionDialog(
collectionPath[collectionPath.length - 1],
collectionPath.slice(0, -1),
windowId,
@@ -117,16 +109,22 @@ export class CollectionDialog extends Component {
setWorkspaceAddVisibility(false);
}
+ /** */
+ dialogContainer() {
+ const { containerId, windowId } = this.props;
+ return document.querySelector(`#${containerId} #${windowId}`);
+ }
+
/** */
placeholder() {
- const { classes, containerId, windowId } = this.props;
+ const { classes } = this.props;
return (
<Dialog
className={classes.dialog}
onClose={this.hideDialog}
open
- container={document.querySelector(`#${containerId} #${windowId}`)}
+ container={this.dialogContainer()}
BackdropProps={this.backdropProps()}
>
<DialogTitle id="select-collection" disableTypography>
@@ -151,18 +149,23 @@ export class CollectionDialog extends Component {
const {
classes,
collection,
- containerId,
error,
isMultipart,
manifest,
ready,
t,
- windowId,
} = this.props;
const { filter } = this.state;
if (error) return null;
+ // If this component is optimistically rendering ahead of the window its in
+ // force a re-render so that it is placed correctly. The right thing here is
+ // to maybe pass a ref.
+ if (!this.dialogContainer()) {
+ this.forceUpdate();
+ return <></>;
+ }
if (!ready) return this.placeholder();
const rights = manifest && (asArray(manifest.getProperty('rights') || manifest.getProperty('license')));
@@ -181,7 +184,7 @@ export class CollectionDialog extends Component {
<Dialog
className={classes.dialog}
onClose={this.hideDialog}
- container={document.querySelector(`#${containerId} #${windowId}`)}
+ container={this.dialogContainer()}
BackdropProps={this.backdropProps()}
open
>
@@ -277,17 +280,14 @@ CollectionDialog.propTypes = {
containerId: PropTypes.string,
error: PropTypes.string,
hideCollectionDialog: PropTypes.func.isRequired,
- hideWindowCollectionDialog: PropTypes.func.isRequired,
isMultipart: PropTypes.bool,
manifest: PropTypes.object.isRequired, // eslint-disable-line react/forbid-prop-types
manifestId: PropTypes.string.isRequired,
ready: PropTypes.bool,
setWorkspaceAddVisibility: PropTypes.func.isRequired,
showCollectionDialog: PropTypes.func.isRequired,
- showWindowCollectionDialog: PropTypes.func.isRequired,
t: PropTypes.func.isRequired,
updateWindow: PropTypes.func.isRequired,
- variant: PropTypes.oneOf(['window', 'workspace']),
windowId: PropTypes.string,
};
@@ -298,6 +298,5 @@ CollectionDialog.defaultProps = {
error: null,
isMultipart: false,
ready: false,
- variant: 'workspace',
windowId: null,
};
diff --git a/src/components/ManifestListItem.js b/src/components/ManifestListItem.js
index 8333ca287..12f8516d2 100644
--- a/src/components/ManifestListItem.js
+++ b/src/components/ManifestListItem.js
@@ -40,16 +40,10 @@ export class ManifestListItem extends React.Component {
addWindow,
handleClose,
manifestId,
- showCollectionDialog,
- isCollection,
} = this.props;
- if (isCollection) {
- showCollectionDialog(manifestId);
- } else {
- addWindow({ manifestId });
- handleClose();
- }
+ addWindow({ manifestId });
+ handleClose();
}
/** */
@@ -185,7 +179,6 @@ ManifestListItem.propTypes = {
manifestLogo: PropTypes.string,
provider: PropTypes.string,
ready: PropTypes.bool,
- showCollectionDialog: PropTypes.func.isRequired,
size: PropTypes.number,
t: PropTypes.func,
thumbnail: PropTypes.string,
diff --git a/src/components/PrimaryWindow.js b/src/components/PrimaryWindow.js
index 331313908..5fc1fa8d0 100644
--- a/src/components/PrimaryWindow.js
+++ b/src/components/PrimaryWindow.js
@@ -34,7 +34,7 @@ export class PrimaryWindow extends Component {
if (isCollection) {
return (
<>
- { isCollectionDialogVisible && <CollectionDialog variant="window" windowId={windowId} /> }
+ { isCollectionDialogVisible && <CollectionDialog windowId={windowId} /> }
<SelectCollection
windowId={windowId}
/>
diff --git a/src/components/SelectCollection.js b/src/components/SelectCollection.js
index c4c8aebde..decfdf6de 100644
--- a/src/components/SelectCollection.js
+++ b/src/components/SelectCollection.js
@@ -19,9 +19,9 @@ export class SelectCollection extends Component {
/** */
openCollectionDialog() {
const {
- collectionPath, manifestId, showWindowCollectionDialog, windowId,
+ collectionPath, manifestId, showCollectionDialog, windowId,
} = this.props;
- showWindowCollectionDialog(manifestId, collectionPath.slice(0, -1), windowId);
+ showCollectionDialog(manifestId, collectionPath.slice(0, -1), windowId);
}
/** */
@@ -54,7 +54,7 @@ export class SelectCollection extends Component {
SelectCollection.propTypes = {
collectionPath: PropTypes.arrayOf(PropTypes.string),
manifestId: PropTypes.string,
- showWindowCollectionDialog: PropTypes.func.isRequired,
+ showCollectionDialog: PropTypes.func.isRequired,
t: PropTypes.func,
windowId: PropTypes.string,
};
diff --git a/src/components/WorkspaceArea.js b/src/components/WorkspaceArea.js
index 672c6a762..3203d39d9 100644
--- a/src/components/WorkspaceArea.js
+++ b/src/components/WorkspaceArea.js
@@ -1,7 +1,6 @@
import React, { Component } from 'react';
import PropTypes from 'prop-types';
import classNames from 'classnames';
-import CollectionDialog from '../containers/CollectionDialog';
import ErrorDialog from '../containers/ErrorDialog';
import WorkspaceControlPanel from '../containers/WorkspaceControlPanel';
import Workspace from '../containers/Workspace';
@@ -22,7 +21,6 @@ export class WorkspaceArea extends Component {
const {
classes,
controlPanelVariant,
- isCollectionDialogVisible,
isWorkspaceAddVisible,
isWorkspaceControlPanelVisible,
lang,
@@ -47,7 +45,6 @@ export class WorkspaceArea extends Component {
}
<ErrorDialog />
<BackgroundPluginArea />
- { isCollectionDialogVisible && <CollectionDialog /> }
</main>
</>
);
@@ -57,7 +54,6 @@ export class WorkspaceArea extends Component {
WorkspaceArea.propTypes = {
classes: PropTypes.objectOf(PropTypes.string).isRequired,
controlPanelVariant: PropTypes.string,
- isCollectionDialogVisible: PropTypes.bool,
isWorkspaceAddVisible: PropTypes.bool,
isWorkspaceControlPanelVisible: PropTypes.bool.isRequired,
lang: PropTypes.string,
@@ -66,7 +62,6 @@ WorkspaceArea.propTypes = {
WorkspaceArea.defaultProps = {
controlPanelVariant: undefined,
- isCollectionDialogVisible: false,
isWorkspaceAddVisible: false,
lang: undefined,
};
diff --git a/src/containers/CollectionDialog.js b/src/containers/CollectionDialog.js
index 21fe169f7..d663a90fa 100644
--- a/src/containers/CollectionDialog.js
+++ b/src/containers/CollectionDialog.js
@@ -17,10 +17,8 @@ import { CollectionDialog } from '../components/CollectionDialog';
const mapDispatchToProps = {
addWindow: actions.addWindow,
hideCollectionDialog: actions.hideCollectionDialog,
- hideWindowCollectionDialog: actions.hideWindowCollectionDialog,
setWorkspaceAddVisibility: actions.setWorkspaceAddVisibility,
showCollectionDialog: actions.showCollectionDialog,
- showWindowCollectionDialog: actions.showWindowCollectionDialog,
updateWindow: actions.updateWindow,
};
@@ -29,10 +27,8 @@ const mapDispatchToProps = {
* @memberof CollectionDialog
* @private
*/
-const mapStateToProps = (state, { variant, windowId }) => {
- const { collectionPath, collectionManifestId: manifestId } = variant === 'window'
- ? getWindow(state, { windowId })
- : state.workspace;
+const mapStateToProps = (state, { windowId }) => {
+ const { collectionPath, collectionManifestId: manifestId } = getWindow(state, { windowId });
const manifest = getManifest(state, { manifestId });
const collectionId = collectionPath && collectionPath[collectionPath.length - 1];
@@ -48,7 +44,7 @@ const mapStateToProps = (state, { variant, windowId }) => {
manifestId,
open: state.workspace.collectionDialogOn,
ready: manifest && !!manifest.json,
- windowId: state.workspace.collectionUpdateWindowId || windowId,
+ windowId,
};
};
diff --git a/src/containers/ManifestListItem.js b/src/containers/ManifestListItem.js
index 20f2db58a..14ca6d70e 100644
--- a/src/containers/ManifestListItem.js
+++ b/src/containers/ManifestListItem.js
@@ -48,7 +48,6 @@ const mapStateToProps = (state, { manifestId, provider }) => {
const mapDispatchToProps = {
addWindow: actions.addWindow,
fetchManifest: actions.fetchManifest,
- showCollectionDialog: actions.showCollectionDialog,
};
/**
diff --git a/src/containers/SelectCollection.js b/src/containers/SelectCollection.js
index a620126da..446c32711 100644
--- a/src/containers/SelectCollection.js
+++ b/src/containers/SelectCollection.js
@@ -20,7 +20,7 @@ const mapStateToProps = (state, { windowId }) => {
};
const mapDispatchToProps = {
- showWindowCollectionDialog: actions.showWindowCollectionDialog,
+ showCollectionDialog: actions.showCollectionDialog,
};
/** */
const styles = (theme) => ({
diff --git a/src/containers/WorkspaceArea.js b/src/containers/WorkspaceArea.js
index a9f7bc2a3..c2655978c 100644
--- a/src/containers/WorkspaceArea.js
+++ b/src/containers/WorkspaceArea.js
@@ -14,7 +14,6 @@ import { getConfig, getWindowIds, getWorkspace } from '../state/selectors';
const mapStateToProps = state => (
{
controlPanelVariant: getWorkspace(state).isWorkspaceAddVisible || getWindowIds(state).length > 0 ? undefined : 'wide',
- isCollectionDialogVisible: getWorkspace(state).collectionDialogOn,
isWorkspaceAddVisible: getWorkspace(state).isWorkspaceAddVisible,
isWorkspaceControlPanelVisible: getConfig(state).workspaceControlPanel.enabled,
lang: getConfig(state).language,
diff --git a/src/state/actions/action-types.js b/src/state/actions/action-types.js
index 69a4ea2a7..c1f124472 100644
--- a/src/state/actions/action-types.js
+++ b/src/state/actions/action-types.js
@@ -72,8 +72,6 @@ const ActionTypes = {
REMOVE_RESOURCE: 'mirador/REMOVE_RESOURCE',
SHOW_COLLECTION_DIALOG: 'mirador/SHOW_COLLECTION_DIALOG',
HIDE_COLLECTION_DIALOG: 'mirador/HIDE_COLLECTION_DIALOG',
- SHOW_WINDOW_COLLECTION_DIALOG: 'mirador/SHOW_WINDOW_COLLECTION_DIALOG',
- HIDE_WINDOW_COLLECTION_DIALOG: 'mirador/HIDE_WINDOW_COLLECTION_DIALOG',
};
export default ActionTypes;
diff --git a/src/state/actions/window.js b/src/state/actions/window.js
index e9f5da200..1534e5403 100644
--- a/src/state/actions/window.js
+++ b/src/state/actions/window.js
@@ -188,19 +188,19 @@ export function setWindowViewType(windowId, viewType) {
}
/** */
-export function showWindowCollectionDialog(manifestId, collectionPath = [], windowId) {
+export function showCollectionDialog(manifestId, collectionPath = [], windowId) {
return {
collectionPath,
manifestId,
- type: ActionTypes.SHOW_WINDOW_COLLECTION_DIALOG,
+ type: ActionTypes.SHOW_COLLECTION_DIALOG,
windowId,
};
}
/** */
-export function hideWindowCollectionDialog(windowId) {
+export function hideCollectionDialog(windowId) {
return {
- type: ActionTypes.HIDE_WINDOW_COLLECTION_DIALOG,
+ type: ActionTypes.HIDE_COLLECTION_DIALOG,
windowId,
};
}
diff --git a/src/state/actions/workspace.js b/src/state/actions/workspace.js
index 29c52e0f1..0d1d8241b 100644
--- a/src/state/actions/workspace.js
+++ b/src/state/actions/workspace.js
@@ -92,20 +92,3 @@ export function toggleDraggingEnabled() {
type: ActionTypes.TOGGLE_DRAGGING,
};
}
-
-/** */
-export function showCollectionDialog(manifestId, collectionPath = [], windowId = null) {
- return {
- collectionPath,
- manifestId,
- type: ActionTypes.SHOW_COLLECTION_DIALOG,
- windowId,
- };
-}
-
-/** */
-export function hideCollectionDialog() {
- return {
- type: ActionTypes.HIDE_COLLECTION_DIALOG,
- };
-}
diff --git a/src/state/reducers/windows.js b/src/state/reducers/windows.js
index 847adbe50..33853fb8c 100644
--- a/src/state/reducers/windows.js
+++ b/src/state/reducers/windows.js
@@ -150,7 +150,7 @@ export const windowsReducer = (state = {}, action) => {
suggestedSearches: undefined,
},
};
- case ActionTypes.SHOW_WINDOW_COLLECTION_DIALOG:
+ case ActionTypes.SHOW_COLLECTION_DIALOG:
return {
...state,
[action.windowId]: {
@@ -160,7 +160,7 @@ export const windowsReducer = (state = {}, action) => {
collectionPath: action.collectionPath,
},
};
- case ActionTypes.HIDE_WINDOW_COLLECTION_DIALOG:
+ case ActionTypes.HIDE_COLLECTION_DIALOG:
return {
...state,
[action.windowId]: {
diff --git a/src/state/reducers/workspace.js b/src/state/reducers/workspace.js
index df6323c58..30d60e4ae 100644
--- a/src/state/reducers/workspace.js
+++ b/src/state/reducers/workspace.js
@@ -99,16 +99,6 @@ export const workspaceReducer = (
return action.state.workspace || {};
case ActionTypes.TOGGLE_DRAGGING:
return { ...state, draggingEnabled: !state.draggingEnabled };
- case ActionTypes.SHOW_COLLECTION_DIALOG:
- return {
- ...state,
- collectionDialogOn: true,
- collectionManifestId: action.manifestId,
- collectionPath: action.collectionPath,
- collectionUpdateWindowId: action.windowId,
- };
- case ActionTypes.HIDE_COLLECTION_DIALOG:
- return { ...state, collectionDialogOn: false };
default:
return state;
}
diff --git a/src/state/sagas/app.js b/src/state/sagas/app.js
index e915f68cf..83a2c4076 100644
--- a/src/state/sagas/app.js
+++ b/src/state/sagas/app.js
@@ -53,6 +53,5 @@ export default function* appSaga() {
takeEvery(ActionTypes.IMPORT_MIRADOR_STATE, importState),
takeEvery(ActionTypes.IMPORT_CONFIG, importConfig),
takeEvery(ActionTypes.SHOW_COLLECTION_DIALOG, fetchCollectionManifests),
- takeEvery(ActionTypes.SHOW_WINDOW_COLLECTION_DIALOG, fetchCollectionManifests),
]);
}
diff --git a/src/state/sagas/windows.js b/src/state/sagas/windows.js
index 57bb042ba..1bfab78d5 100644
--- a/src/state/sagas/windows.js
+++ b/src/state/sagas/windows.js
@@ -13,7 +13,7 @@ import {
fetchSearch,
receiveManifest,
fetchInfoResponse,
- showWindowCollectionDialog,
+ showCollectionDialog,
} from '../actions';
import {
getSearchForWindow, getSearchAnnotationsForCompanionWindow,
@@ -236,7 +236,7 @@ export function* fetchInfoResponses({ visibleCanvases: visibleCanvasIds, windowI
export function* determineAndShowCollectionDialog(manifestId, windowId) {
const manifestoInstance = yield select(getManifestoInstance, { manifestId });
if (manifestoInstance && manifestoInstance.isCollection()) {
- yield put(showWindowCollectionDialog(manifestId, [], windowId));
+ yield put(showCollectionDialog(manifestId, [], windowId));
}
}
--
GitLab