From 46bd3bcc6208774b165cfa91d17e69eabcd7aa3e Mon Sep 17 00:00:00 2001
From: Jessie Keck <jkeck@stanford.edu>
Date: Wed, 20 Mar 2019 18:34:48 -0700
Subject: [PATCH] Fix a few issues around Tooltips (#2272)
* Add a container around the MiradorMenuButton so it can get the container ID to be passed to the Popper.
* Allow for TooltipProps to be sent into the MiradorMenuButton component.
* Pass some styling TooltipProps to the MiradorMenuButton to absolutely position the tooltip.
---
.../src/components/CompanionArea.test.js | 17 +++++------
.../src/components/CompanionWindow.test.js | 9 +++---
.../src/components/MiradorMenuButton.test.js | 8 +++++-
__tests__/src/components/WindowTopBar.test.js | 2 +-
.../components/WindowTopMenuButton.test.js | 2 +-
.../WorkspaceFullScreenButton.test.js | 7 +++--
.../components/WorkspaceMenuButton.test.js | 11 ++++----
__tests__/src/components/ZoomControls.test.js | 3 +-
src/components/CompanionArea.js | 3 +-
src/components/CompanionWindow.js | 2 +-
src/components/MiradorMenuButton.js | 28 ++++++++++++++++---
src/components/ViewerNavigation.js | 2 +-
src/components/WindowTopBar.js | 2 +-
src/components/WindowTopMenuButton.js | 2 +-
src/components/WorkspaceAdd.js | 2 +-
src/components/WorkspaceFullScreenButton.js | 2 +-
src/components/WorkspaceMenuButton.js | 2 +-
src/components/ZoomControls.js | 2 +-
src/containers/CompanionArea.js | 1 -
src/containers/MiradorMenuButton.js | 16 +++++++++++
20 files changed, 85 insertions(+), 38 deletions(-)
create mode 100644 src/containers/MiradorMenuButton.js
diff --git a/__tests__/src/components/CompanionArea.test.js b/__tests__/src/components/CompanionArea.test.js
index f896ddb43..87382872e 100644
--- a/__tests__/src/components/CompanionArea.test.js
+++ b/__tests__/src/components/CompanionArea.test.js
@@ -1,5 +1,6 @@
import React from 'react';
import { shallow } from 'enzyme';
+import MiradorMenuButton from '../../../src/containers/MiradorMenuButton';
import { CompanionArea } from '../../../src/components/CompanionArea';
import CompanionWindowFactory from '../../../src/containers/CompanionWindowFactory';
@@ -45,13 +46,13 @@ describe('CompanionArea', () => {
position: 'left', sideBarOpen: true, setCompanionAreaOpen, companionAreaOpen: false,
});
- expect(wrapper.find('MiradorMenuButton').length).toBe(1);
- expect(wrapper.find('MiradorMenuButton').first().children('pure(ArrowRightSharpIcon)').length).toBe(1);
+ expect(wrapper.find(MiradorMenuButton).length).toBe(1);
+ expect(wrapper.find(MiradorMenuButton).first().children('pure(ArrowRightSharpIcon)').length).toBe(1);
expect(wrapper.find('div.mirador-companion-windows').length).toBe(1);
expect(wrapper.find('div.mirador-companion-windows').props().style.display).toBe('none');
- wrapper.find('MiradorMenuButton').first().props().onClick(); // Trigger the onClick prop
+ wrapper.find(MiradorMenuButton).first().props().onClick(); // Trigger the onClick prop
expect(setCompanionAreaOpen).toHaveBeenCalledWith('abc123', true);
});
@@ -63,13 +64,13 @@ describe('CompanionArea', () => {
position: 'left', sideBarOpen: true, setCompanionAreaOpen, companionAreaOpen: true,
});
- expect(wrapper.find('MiradorMenuButton').length).toBe(1);
- expect(wrapper.find('MiradorMenuButton').first().children('pure(ArrowLeftSharpIcon)').length).toBe(1);
+ expect(wrapper.find(MiradorMenuButton).length).toBe(1);
+ expect(wrapper.find(MiradorMenuButton).first().children('pure(ArrowLeftSharpIcon)').length).toBe(1);
expect(wrapper.find('div.mirador-companion-windows').length).toBe(1);
expect(wrapper.find('div.mirador-companion-windows').props().style.display).toBe('flex');
- wrapper.find('MiradorMenuButton').first().props().onClick(); // Trigger the onClick prop
+ wrapper.find(MiradorMenuButton).first().props().onClick(); // Trigger the onClick prop
expect(setCompanionAreaOpen).toHaveBeenCalledWith('abc123', false);
});
@@ -79,7 +80,7 @@ describe('CompanionArea', () => {
position: 'left', sideBarOpen: false, setCompanionAreaOpen: () => {}, companionAreaOpen: true,
});
- expect(wrapper.find('MiradorMenuButton').length).toBe(0);
+ expect(wrapper.find(MiradorMenuButton).length).toBe(0);
});
it('does not show a toggle in other positions', () => {
@@ -87,6 +88,6 @@ describe('CompanionArea', () => {
position: 'whatever', sideBarOpen: true, setCompanionAreaOpen: () => {}, companionAreaOpen: true,
});
- expect(wrapper.find('MiradorMenuButton').length).toBe(0);
+ expect(wrapper.find(MiradorMenuButton).length).toBe(0);
});
});
diff --git a/__tests__/src/components/CompanionWindow.test.js b/__tests__/src/components/CompanionWindow.test.js
index 632362db1..c69f9291e 100644
--- a/__tests__/src/components/CompanionWindow.test.js
+++ b/__tests__/src/components/CompanionWindow.test.js
@@ -1,5 +1,6 @@
import React from 'react';
import { shallow } from 'enzyme';
+import MiradorMenuButton from '../../../src/containers/MiradorMenuButton';
import { CompanionWindow } from '../../../src/components/CompanionWindow';
/** create wrapper */
@@ -27,7 +28,7 @@ describe('CompanionWindow', () => {
position: 'left',
});
- const button = companionWindow.find('MiradorMenuButton');
+ const button = companionWindow.find(MiradorMenuButton);
button.props().onClick(); // Trigger the onClick prop
expect(updateCompanionWindow).toHaveBeenCalledTimes(1);
expect(updateCompanionWindow).toHaveBeenCalledWith('x', 'abc123', { position: 'right' });
@@ -41,7 +42,7 @@ describe('CompanionWindow', () => {
onCloseClick: removeCompanionWindowEvent,
});
- const button = companionWindow.find('MiradorMenuButton');
+ const button = companionWindow.find(MiradorMenuButton);
button.props().onClick(); // Trigger the onClick prop
expect(removeCompanionWindowEvent).toHaveBeenCalledTimes(1);
});
@@ -53,7 +54,7 @@ describe('CompanionWindow', () => {
expect(companionWindow.find('WithStyles(Paper).vertical').length).toBe(1);
- const button = companionWindow.find('MiradorMenuButton').first();
+ const button = companionWindow.find(MiradorMenuButton).first();
button.props().onClick(); // Trigger the onClick prop
expect(updateCompanionWindow).toHaveBeenCalledTimes(1);
expect(updateCompanionWindow).toHaveBeenCalledWith('x', 'abc123', { position: 'bottom' });
@@ -65,7 +66,7 @@ describe('CompanionWindow', () => {
expect(companionWindow.find('WithStyles(Paper).horizontal').length).toBe(1);
- const button = companionWindow.find('MiradorMenuButton').first();
+ const button = companionWindow.find(MiradorMenuButton).first();
button.props().onClick(); // Trigger the onClick prop
expect(updateCompanionWindow).toHaveBeenCalledTimes(1);
expect(updateCompanionWindow).toHaveBeenCalledWith('x', 'abc123', { position: 'right' });
diff --git a/__tests__/src/components/MiradorMenuButton.test.js b/__tests__/src/components/MiradorMenuButton.test.js
index f993d4af2..816b67e5b 100644
--- a/__tests__/src/components/MiradorMenuButton.test.js
+++ b/__tests__/src/components/MiradorMenuButton.test.js
@@ -7,7 +7,7 @@ import { MiradorMenuButton } from '../../../src/components/MiradorMenuButton';
*/
function createWrapper(props) {
return shallow(
- <MiradorMenuButton aria-label="The Label" {...props}>
+ <MiradorMenuButton aria-label="The Label" containerId="mirador" {...props}>
<>icon</>
</MiradorMenuButton>,
);
@@ -42,6 +42,12 @@ describe('MiradorMenuButton', () => {
expect(wrapper.find('WithStyles(Tooltip) span').props().className).toEqual('someClass');
});
+ it('spreads TooltipProps to the Tooltip component', () => {
+ wrapper = createWrapper({ TooltipProps: { style: { color: 'red' } } });
+
+ expect(wrapper.find('WithStyles(Tooltip)').props().style).toEqual({ color: 'red' });
+ });
+
it('spreads any other props to IconButton', () => {
wrapper = createWrapper({ color: 'inherit' });
diff --git a/__tests__/src/components/WindowTopBar.test.js b/__tests__/src/components/WindowTopBar.test.js
index 4706ceefc..14085b7bd 100644
--- a/__tests__/src/components/WindowTopBar.test.js
+++ b/__tests__/src/components/WindowTopBar.test.js
@@ -7,7 +7,7 @@ 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 MiradorMenuButton from '../../../src/containers/MiradorMenuButton';
import { WindowTopBar } from '../../../src/components/WindowTopBar';
/** create wrapper */
diff --git a/__tests__/src/components/WindowTopMenuButton.test.js b/__tests__/src/components/WindowTopMenuButton.test.js
index a797287ba..37c147c04 100644
--- a/__tests__/src/components/WindowTopMenuButton.test.js
+++ b/__tests__/src/components/WindowTopMenuButton.test.js
@@ -2,7 +2,7 @@ import React from 'react';
import { shallow } from 'enzyme';
import WindowTopMenu from '../../../src/containers/WindowTopMenu';
import { WindowTopMenuButton } from '../../../src/components/WindowTopMenuButton';
-import { MiradorMenuButton } from '../../../src/components/MiradorMenuButton';
+import MiradorMenuButton from '../../../src/containers/MiradorMenuButton';
/** create wrapper */
function createWrapper(props) {
diff --git a/__tests__/src/components/WorkspaceFullScreenButton.test.js b/__tests__/src/components/WorkspaceFullScreenButton.test.js
index 1981f9ce6..41e8f6371 100644
--- a/__tests__/src/components/WorkspaceFullScreenButton.test.js
+++ b/__tests__/src/components/WorkspaceFullScreenButton.test.js
@@ -1,5 +1,6 @@
import React from 'react';
import { shallow } from 'enzyme';
+import MiradorMenuButton from '../../../src/containers/MiradorMenuButton';
import { WorkspaceFullScreenButton } from '../../../src/components/WorkspaceFullScreenButton';
/** */
@@ -21,7 +22,7 @@ describe('WorkspaceFullScreenButton', () => {
it('renders without an error', () => {
wrapper = createWrapper();
- expect(wrapper.find('MiradorMenuButton').length).toBe(1);
+ expect(wrapper.find(MiradorMenuButton).length).toBe(1);
});
describe('when not in fullscreen', () => {
@@ -29,7 +30,7 @@ describe('WorkspaceFullScreenButton', () => {
beforeAll(() => {
setWorkspaceFullscreen = jest.fn();
wrapper = createWrapper({ setWorkspaceFullscreen });
- menuButton = wrapper.find('MiradorMenuButton');
+ menuButton = wrapper.find(MiradorMenuButton);
});
it('has the FullscreenIcon', () => {
@@ -51,7 +52,7 @@ describe('WorkspaceFullScreenButton', () => {
beforeAll(() => {
setWorkspaceFullscreen = jest.fn();
wrapper = createWrapper({ setWorkspaceFullscreen, isFullscreenEnabled: true });
- menuButton = wrapper.find('MiradorMenuButton');
+ menuButton = wrapper.find(MiradorMenuButton);
});
it('has the FullscreenExitIcon', () => {
diff --git a/__tests__/src/components/WorkspaceMenuButton.test.js b/__tests__/src/components/WorkspaceMenuButton.test.js
index 879ffd878..219c39b10 100644
--- a/__tests__/src/components/WorkspaceMenuButton.test.js
+++ b/__tests__/src/components/WorkspaceMenuButton.test.js
@@ -1,5 +1,6 @@
import React from 'react';
import { shallow } from 'enzyme';
+import MiradorMenuButton from '../../../src/containers/MiradorMenuButton';
import { WorkspaceMenuButton } from '../../../src/components/WorkspaceMenuButton';
describe('WorkspaceMenuButton', () => {
@@ -11,16 +12,16 @@ describe('WorkspaceMenuButton', () => {
});
it('renders without an error', () => {
- expect(wrapper.find('MiradorMenuButton').length).toBe(1);
+ expect(wrapper.find(MiradorMenuButton).length).toBe(1);
});
it('the button has a class indicating that it is "selected" once it is clicked', () => {
- const menuButton = wrapper.find('MiradorMenuButton').first();
+ const menuButton = wrapper.find(MiradorMenuButton).first();
- expect(wrapper.find('MiradorMenuButton').first().props().className).toEqual('');
+ expect(wrapper.find(MiradorMenuButton).first().props().className).toEqual('');
menuButton.props().onClick({ currentTarget: 'anElement' });
- expect(wrapper.find('MiradorMenuButton').first().props().className).toEqual('ctrlBtnSelected');
+ 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('');
});
});
diff --git a/__tests__/src/components/ZoomControls.test.js b/__tests__/src/components/ZoomControls.test.js
index 98d792acb..27ecd4ffc 100644
--- a/__tests__/src/components/ZoomControls.test.js
+++ b/__tests__/src/components/ZoomControls.test.js
@@ -1,5 +1,6 @@
import React from 'react';
import { shallow } from 'enzyme';
+import MiradorMenuButton from '../../../src/containers/MiradorMenuButton';
import { ZoomControls } from '../../../src/components/ZoomControls';
describe('ZoomControls', () => {
@@ -47,7 +48,7 @@ describe('ZoomControls', () => {
it('renders a couple buttons', () => {
expect(wrapper.find('div.zoom_controls').length).toBe(1);
- expect(wrapper.find('MiradorMenuButton').length).toBe(3);
+ expect(wrapper.find(MiradorMenuButton).length).toBe(3);
});
it('has a zoom-in button', () => {
diff --git a/src/components/CompanionArea.js b/src/components/CompanionArea.js
index 15d4f7ac5..3a00400a7 100644
--- a/src/components/CompanionArea.js
+++ b/src/components/CompanionArea.js
@@ -3,7 +3,7 @@ import PropTypes from 'prop-types';
import ArrowLeftIcon from '@material-ui/icons/ArrowLeftSharp';
import ArrowRightIcon from '@material-ui/icons/ArrowRightSharp';
import CompanionWindowFactory from '../containers/CompanionWindowFactory';
-import { MiradorMenuButton } from './MiradorMenuButton';
+import MiradorMenuButton from '../containers/MiradorMenuButton';
import ns from '../config/css-ns';
/** */
@@ -33,6 +33,7 @@ export class CompanionArea extends Component {
aria-label={companionAreaOpen ? t('collapseSidePanel') : t('expandSidePanel')}
className={classes.toggle}
onClick={() => { setCompanionAreaOpen(windowId, !companionAreaOpen); }}
+ TooltipProps={{ style: { right: '0', position: 'absolute' } }}
>
{companionAreaOpen ? <ArrowLeftIcon /> : <ArrowRightIcon />}
</MiradorMenuButton>
diff --git a/src/components/CompanionWindow.js b/src/components/CompanionWindow.js
index 620a5e26d..0fc4093f5 100644
--- a/src/components/CompanionWindow.js
+++ b/src/components/CompanionWindow.js
@@ -7,7 +7,7 @@ import Typography from '@material-ui/core/Typography';
import Toolbar from '@material-ui/core/Toolbar';
import ThumbnailNavigationBottomIcon from './icons/ThumbnailNavigationBottomIcon';
import ThumbnailNavigationRightIcon from './icons/ThumbnailNavigationRightIcon';
-import { MiradorMenuButton } from './MiradorMenuButton';
+import MiradorMenuButton from '../containers/MiradorMenuButton';
import ns from '../config/css-ns';
/**
diff --git a/src/components/MiradorMenuButton.js b/src/components/MiradorMenuButton.js
index 40d9564e7..8b3bf1e52 100644
--- a/src/components/MiradorMenuButton.js
+++ b/src/components/MiradorMenuButton.js
@@ -2,7 +2,7 @@ import React from 'react';
import PropTypes from 'prop-types';
import IconButton from '@material-ui/core/IconButton';
import Tooltip from '@material-ui/core/Tooltip';
-
+import ns from '../config/css-ns';
/**
* MiradorMenuButton ~ Wrap the given icon prop in an IconButton and a Tooltip.
@@ -11,16 +11,31 @@ import Tooltip from '@material-ui/core/Tooltip';
*/
export function MiradorMenuButton(props) {
const { 'aria-label': ariaLabel } = props;
- const { children, wrapperClassName, ...iconButtonProps } = props;
+ const {
+ children,
+ containerId,
+ dispatch,
+ TooltipProps,
+ wrapperClassName,
+ ...iconButtonProps
+ } = props;
return (
- <Tooltip title={ariaLabel}>
+ <Tooltip
+ PopperProps={{
+ container: document.querySelector(`#${containerId} .${ns('viewer')}`),
+ }}
+ title={ariaLabel}
+ {...TooltipProps}
+ >
{/*
Wrap IconButton in span so it can receive mouse events
(e.g. show the tooltip) even if the IconButton is disabled
*/}
<span className={wrapperClassName}>
- <IconButton {...iconButtonProps}>{children}</IconButton>
+ <IconButton {...iconButtonProps}>
+ {children}
+ </IconButton>
</span>
</Tooltip>
);
@@ -29,9 +44,14 @@ export function MiradorMenuButton(props) {
MiradorMenuButton.propTypes = {
'aria-label': PropTypes.string.isRequired,
children: PropTypes.element.isRequired,
+ containerId: PropTypes.string.isRequired,
+ dispatch: PropTypes.func,
+ TooltipProps: PropTypes.object, // eslint-disable-line react/forbid-prop-types
wrapperClassName: PropTypes.string,
};
MiradorMenuButton.defaultProps = {
+ dispatch: () => {},
+ TooltipProps: {},
wrapperClassName: null,
};
diff --git a/src/components/ViewerNavigation.js b/src/components/ViewerNavigation.js
index de84484d1..00d47a7c6 100644
--- a/src/components/ViewerNavigation.js
+++ b/src/components/ViewerNavigation.js
@@ -1,7 +1,7 @@
import React, { Component } from 'react';
import NavigationIcon from '@material-ui/icons/PlayCircleOutlineSharp';
import PropTypes from 'prop-types';
-import { MiradorMenuButton } from './MiradorMenuButton';
+import MiradorMenuButton from '../containers/MiradorMenuButton';
import ns from '../config/css-ns';
/**
diff --git a/src/components/WindowTopBar.js b/src/components/WindowTopBar.js
index 0dfdce86f..739611b05 100644
--- a/src/components/WindowTopBar.js
+++ b/src/components/WindowTopBar.js
@@ -10,7 +10,7 @@ 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 MiradorMenuButton from '../containers/MiradorMenuButton';
import ns from '../config/css-ns';
diff --git a/src/components/WindowTopMenuButton.js b/src/components/WindowTopMenuButton.js
index 901d6349c..94df82124 100644
--- a/src/components/WindowTopMenuButton.js
+++ b/src/components/WindowTopMenuButton.js
@@ -2,7 +2,7 @@ import React, { Component } from 'react';
import MoreVertIcon from '@material-ui/icons/MoreVertSharp';
import PropTypes from 'prop-types';
import WindowTopMenu from '../containers/WindowTopMenu';
-import { MiradorMenuButton } from './MiradorMenuButton';
+import MiradorMenuButton from '../containers/MiradorMenuButton';
/**
*/
diff --git a/src/components/WorkspaceAdd.js b/src/components/WorkspaceAdd.js
index da05970f6..5b78d76d8 100644
--- a/src/components/WorkspaceAdd.js
+++ b/src/components/WorkspaceAdd.js
@@ -13,7 +13,7 @@ import Typography from '@material-ui/core/Typography';
import ns from '../config/css-ns';
import ManifestForm from '../containers/ManifestForm';
import ManifestListItem from '../containers/ManifestListItem';
-import { MiradorMenuButton } from './MiradorMenuButton';
+import MiradorMenuButton from '../containers/MiradorMenuButton';
/**
* An area for managing manifests and adding them to workspace
diff --git a/src/components/WorkspaceFullScreenButton.js b/src/components/WorkspaceFullScreenButton.js
index 09fd55281..20b74c455 100644
--- a/src/components/WorkspaceFullScreenButton.js
+++ b/src/components/WorkspaceFullScreenButton.js
@@ -2,7 +2,7 @@ import React, { Component } from 'react';
import FullscreenIcon from '@material-ui/icons/FullscreenSharp';
import FullscreenExitIcon from '@material-ui/icons/FullscreenExitSharp';
import PropTypes from 'prop-types';
-import { MiradorMenuButton } from './MiradorMenuButton';
+import MiradorMenuButton from '../containers/MiradorMenuButton';
/**
*/
export class WorkspaceFullScreenButton extends Component {
diff --git a/src/components/WorkspaceMenuButton.js b/src/components/WorkspaceMenuButton.js
index 18980d48b..94bab4213 100644
--- a/src/components/WorkspaceMenuButton.js
+++ b/src/components/WorkspaceMenuButton.js
@@ -3,7 +3,7 @@ import MenuIcon from '@material-ui/icons/MenuSharp';
import PropTypes from 'prop-types';
import classNames from 'classnames';
import WorkspaceMenu from '../containers/WorkspaceMenu';
-import { MiradorMenuButton } from './MiradorMenuButton';
+import MiradorMenuButton from '../containers/MiradorMenuButton';
/**
*/
diff --git a/src/components/ZoomControls.js b/src/components/ZoomControls.js
index f96c52a4e..71e1d7702 100644
--- a/src/components/ZoomControls.js
+++ b/src/components/ZoomControls.js
@@ -3,7 +3,7 @@ import AddCircleIcon from '@material-ui/icons/AddCircleOutlineSharp';
import RemoveCircleIcon from '@material-ui/icons/RemoveCircleOutlineSharp';
import PropTypes from 'prop-types';
import RestoreZoomIcon from './icons/RestoreZoomIcon';
-import { MiradorMenuButton } from './MiradorMenuButton';
+import MiradorMenuButton from '../containers/MiradorMenuButton';
/**
*/
diff --git a/src/containers/CompanionArea.js b/src/containers/CompanionArea.js
index f9d577e24..3eacbb201 100644
--- a/src/containers/CompanionArea.js
+++ b/src/containers/CompanionArea.js
@@ -32,7 +32,6 @@ const styles = theme => ({
},
toggle: {
position: 'absolute',
- left: '100%',
width: '1rem',
zIndex: theme.zIndex.drawer,
backgroundColor: theme.palette.background.paper,
diff --git a/src/containers/MiradorMenuButton.js b/src/containers/MiradorMenuButton.js
new file mode 100644
index 000000000..7cb1b3a00
--- /dev/null
+++ b/src/containers/MiradorMenuButton.js
@@ -0,0 +1,16 @@
+import { compose } from 'redux';
+import { connect } from 'react-redux';
+import { withPlugins } from '../extend';
+import { MiradorMenuButton } from '../components/MiradorMenuButton';
+
+/** */
+const mapStateToProps = state => ({
+ containerId: state.config.id,
+});
+
+const enhance = compose(
+ connect(mapStateToProps, null),
+ withPlugins('MiradorMenuButton'),
+);
+
+export default enhance(MiradorMenuButton);
--
GitLab