From bf90b255e06dbe28e7f44e90b069f7f54f9dc262 Mon Sep 17 00:00:00 2001
From: Christopher Hanna Johnson <chjohnson39@gmail.com>
Date: Sun, 17 Mar 2019 10:40:13 +0100
Subject: [PATCH] optimizes and corrects dense variant WindowTopBar and button
styling removes conflicting CSS button class bindings and duplicate default
declarations part of #2214
---
.../mirador/window_actions.test.js | 2 +-
__tests__/src/components/WindowTopBar.test.js | 7 -------
.../components/WorkspaceMenuButton.test.js | 3 +--
src/components/WindowTopBar.js | 21 +++++++++----------
src/components/WindowTopMenuButton.js | 3 +--
src/components/WorkspaceFullScreenButton.js | 4 +---
src/components/WorkspaceMenuButton.js | 2 +-
src/containers/WindowTopMenuButton.js | 3 ---
src/containers/WorkspaceFullScreenButton.js | 12 -----------
src/containers/WorkspaceMenuButton.js | 3 ---
10 files changed, 15 insertions(+), 45 deletions(-)
diff --git a/__tests__/integration/mirador/window_actions.test.js b/__tests__/integration/mirador/window_actions.test.js
index d7c892b22..538b80174 100644
--- a/__tests__/integration/mirador/window_actions.test.js
+++ b/__tests__/integration/mirador/window_actions.test.js
@@ -13,7 +13,7 @@ describe('Window actions', () => {
await expect(page).toMatchElement('.mirador-window');
await page.waitFor(1000);
- await expect(page).toClick('.mirador-window-close');
+ await expect(page).toClick('button[aria-label="Close window"]');
const numWindows = await page.evaluate(page => (
document.querySelectorAll('.mirador-window').length
)); // only default configed windows found
diff --git a/__tests__/src/components/WindowTopBar.test.js b/__tests__/src/components/WindowTopBar.test.js
index 51ad72784..e79197344 100644
--- a/__tests__/src/components/WindowTopBar.test.js
+++ b/__tests__/src/components/WindowTopBar.test.js
@@ -6,7 +6,6 @@ import Toolbar from '@material-ui/core/Toolbar';
import AppBar from '@material-ui/core/AppBar';
import WindowTopMenuButton from '../../../src/containers/WindowTopMenuButton';
-import WindowTopBarButtons from '../../../src/containers/WindowTopBarButtons';
import { MiradorMenuButton } from '../../../src/components/MiradorMenuButton';
import { WindowTopBar } from '../../../src/components/WindowTopBar';
@@ -35,7 +34,6 @@ describe('WindowTopBar', () => {
expect(wrapper.find(Toolbar).length).toBe(1);
expect(wrapper.find(MiradorMenuButton).length).toBe(3);
expect(wrapper.find(Typography).length).toBe(1);
- expect(wrapper.find(WindowTopBarButtons).length).toBe(1);
expect(wrapper.find(WindowTopMenuButton).length).toBe(1);
});
@@ -50,11 +48,6 @@ describe('WindowTopBar', () => {
expect(wrapper.find(Typography).first().render().text()).toBe('awesome manifest');
});
- it('passes correct props to <WindowTopBarButtons/>', () => {
- const wrapper = createWrapper();
- expect(wrapper.find(WindowTopBarButtons).first().props().windowId).toBe('xyz');
- });
-
it('passe correct props to <WindowTopMenuButton', () => {
const wrapper = createWrapper();
expect(wrapper.find(WindowTopMenuButton).first().props().windowId).toBe('xyz');
diff --git a/__tests__/src/components/WorkspaceMenuButton.test.js b/__tests__/src/components/WorkspaceMenuButton.test.js
index 879ffd878..97b064ca7 100644
--- a/__tests__/src/components/WorkspaceMenuButton.test.js
+++ b/__tests__/src/components/WorkspaceMenuButton.test.js
@@ -17,10 +17,9 @@ describe('WorkspaceMenuButton', () => {
it('the button has a class indicating that it is "selected" once it is clicked', () => {
const menuButton = wrapper.find('MiradorMenuButton').first();
- expect(wrapper.find('MiradorMenuButton').first().props().className).toEqual('');
menuButton.props().onClick({ currentTarget: 'anElement' });
expect(wrapper.find('MiradorMenuButton').first().props().className).toEqual('ctrlBtnSelected');
menuButton.props().onClick({});
- expect(wrapper.find('MiradorMenuButton').first().props().className).toEqual('');
+ expect(wrapper.find('MiradorMenuButton').first().props().className).toEqual(null);
});
});
diff --git a/src/components/WindowTopBar.js b/src/components/WindowTopBar.js
index 3a25435d4..7b38013bb 100644
--- a/src/components/WindowTopBar.js
+++ b/src/components/WindowTopBar.js
@@ -9,7 +9,6 @@ import Toolbar from '@material-ui/core/Toolbar';
import AppBar from '@material-ui/core/AppBar';
import classNames from 'classnames';
import WindowTopMenuButton from '../containers/WindowTopMenuButton';
-import WindowTopBarButtons from '../containers/WindowTopBarButtons';
import { MiradorMenuButton } from './MiradorMenuButton';
import ns from '../config/css-ns';
@@ -28,32 +27,32 @@ export class WindowTopBar extends Component {
maximizeWindow, maximized, minimizeWindow, focused,
} = this.props;
return (
- <AppBar position="relative">
- <Toolbar disableGutters className={classNames(classes.windowTopBarStyle, focused ? classes.focused : null, ns('window-top-bar'))} variant="dense">
+ <AppBar
+ className={classNames(classes.windowTopBarStyle, focused ? classes.focused : null, ns('window-top-bar'))}
+ color="secondary"
+ position="static"
+ >
+ <Toolbar disableGutters variant="dense">
<MiradorMenuButton
aria-label={t('toggleWindowSideBar')}
- color="inherit"
onClick={toggleWindowSideBar}
>
<MenuIcon />
</MiradorMenuButton>
- <Typography variant="h2" noWrap color="inherit" className={classes.title}>
+ <Typography variant="h2" noWrap className={classes.title}>
{manifestTitle}
</Typography>
- <WindowTopBarButtons windowId={windowId} />
- <WindowTopMenuButton className={ns('window-menu-btn')} windowId={windowId} />
+ <WindowTopMenuButton
+ windowId={windowId}
+ />
<MiradorMenuButton
aria-label={(maximized ? t('minimizeWindow') : t('maximizeWindow'))}
- className={ns('window-maximize')}
- color="inherit"
onClick={(maximized ? minimizeWindow : maximizeWindow)}
>
{(maximized ? <FullscreenExitIcon /> : <FullscreenIcon />)}
</MiradorMenuButton>
<MiradorMenuButton
aria-label={t('closeWindow')}
- className={ns('window-close')}
- color="inherit"
onClick={removeWindow}
>
<CloseIcon />
diff --git a/src/components/WindowTopMenuButton.js b/src/components/WindowTopMenuButton.js
index 61dd7091f..06b653f49 100644
--- a/src/components/WindowTopMenuButton.js
+++ b/src/components/WindowTopMenuButton.js
@@ -52,8 +52,7 @@ export class WindowTopMenuButton extends Component {
aria-haspopup="true"
aria-label={t('windowMenu')}
aria-owns={anchorEl ? `window-menu_${windowId}` : undefined}
- className={classNames(classes.ctrlBtn, (anchorEl ? classes.ctrlBtnSelected : null))}
- color="inherit"
+ className={classNames(anchorEl ? classes.ctrlBtnSelected : null)}
onClick={this.handleMenuClick}
>
<MoreVertIcon />
diff --git a/src/components/WorkspaceFullScreenButton.js b/src/components/WorkspaceFullScreenButton.js
index 09fd55281..f990a4b48 100644
--- a/src/components/WorkspaceFullScreenButton.js
+++ b/src/components/WorkspaceFullScreenButton.js
@@ -12,12 +12,11 @@ export class WorkspaceFullScreenButton extends Component {
*/
render() {
const {
- classes, isFullscreenEnabled, setWorkspaceFullscreen, t,
+ isFullscreenEnabled, setWorkspaceFullscreen, t,
} = this.props;
return (
<MiradorMenuButton
aria-label={isFullscreenEnabled ? t('exitFullScreen') : t('workspaceFullScreen')}
- className={classes.ctrlBtn}
onClick={() => setWorkspaceFullscreen(!isFullscreenEnabled)}
>
{isFullscreenEnabled ? <FullscreenExitIcon /> : <FullscreenIcon />}
@@ -29,7 +28,6 @@ export class WorkspaceFullScreenButton extends Component {
WorkspaceFullScreenButton.propTypes = {
isFullscreenEnabled: PropTypes.bool,
setWorkspaceFullscreen: PropTypes.func.isRequired,
- classes: PropTypes.object.isRequired, // eslint-disable-line react/forbid-prop-types
t: PropTypes.func,
};
diff --git a/src/components/WorkspaceMenuButton.js b/src/components/WorkspaceMenuButton.js
index 18980d48b..9b5eebc16 100644
--- a/src/components/WorkspaceMenuButton.js
+++ b/src/components/WorkspaceMenuButton.js
@@ -52,7 +52,7 @@ export class WorkspaceMenuButton extends Component {
aria-haspopup="true"
aria-label={t('workspaceMenu')}
aria-owns={anchorEl ? 'workspace-menu' : undefined}
- className={classNames(classes.ctrlBtn, (anchorEl ? classes.ctrlBtnSelected : null))}
+ className={anchorEl ? classes.ctrlBtnSelected : null}
id="menuBtn"
onClick={this.handleMenuClick}
>
diff --git a/src/containers/WindowTopMenuButton.js b/src/containers/WindowTopMenuButton.js
index bcef8e986..24e4922cd 100644
--- a/src/containers/WindowTopMenuButton.js
+++ b/src/containers/WindowTopMenuButton.js
@@ -9,9 +9,6 @@ import { WindowTopMenuButton } from '../components/WindowTopMenuButton';
* @returns {{ctrlBtn: {margin: (number|string)}}}
*/
const styles = theme => ({
- ctrlBtn: {
- margin: theme.spacing.unit,
- },
ctrlBtnSelected: {
backgroundColor: theme.palette.action.selected,
},
diff --git a/src/containers/WorkspaceFullScreenButton.js b/src/containers/WorkspaceFullScreenButton.js
index adb534737..46f8cf9c0 100644
--- a/src/containers/WorkspaceFullScreenButton.js
+++ b/src/containers/WorkspaceFullScreenButton.js
@@ -22,20 +22,8 @@ const mapStateToProps = state => ({
*/
const mapDispatchToProps = { setWorkspaceFullscreen: actions.setWorkspaceFullscreen };
-/**
- *
- * @param theme
- * @returns {{ctrlBtn: {margin: (number|string)}}}
- */
-const styles = theme => ({
- ctrlBtn: {
- margin: theme.spacing.unit,
- },
-});
-
const enhance = compose(
withTranslation(),
- withStyles(styles),
connect(mapStateToProps, mapDispatchToProps),
);
diff --git a/src/containers/WorkspaceMenuButton.js b/src/containers/WorkspaceMenuButton.js
index a7f928e67..e42ec015b 100644
--- a/src/containers/WorkspaceMenuButton.js
+++ b/src/containers/WorkspaceMenuButton.js
@@ -9,9 +9,6 @@ import { WorkspaceMenuButton } from '../components/WorkspaceMenuButton';
* @returns {{ctrlBtn: {margin: (number|string)}}}
*/
const styles = theme => ({
- ctrlBtn: {
- margin: theme.spacing.unit,
- },
ctrlBtnSelected: {
backgroundColor: theme.palette.action.selected,
},
--
GitLab