diff --git a/__tests__/src/components/WindowThumbnailSettings.test.js b/__tests__/src/components/WindowThumbnailSettings.test.js index a91b88e0a..9112981a1 100644 --- a/__tests__/src/components/WindowThumbnailSettings.test.js +++ b/__tests__/src/components/WindowThumbnailSettings.test.js @@ -20,28 +20,28 @@ describe('WindowThumbnailSettings', () => { it('renders all elements correctly', () => { createWrapper(); expect(screen.getByRole('presentation', { selector: 'li' })).toBeInTheDocument(); - expect(screen.getByRole('menuitem', { name: /Off/ })).toBeInTheDocument(); - expect(screen.getByRole('menuitem', { name: /Bottom/ })).toBeInTheDocument(); - expect(screen.getByRole('menuitem', { name: /Right/ })).toBeInTheDocument(); + expect(screen.getByRole('menuitemradio', { name: /Off/ })).toBeInTheDocument(); + expect(screen.getByRole('menuitemradio', { name: /Bottom/ })).toBeInTheDocument(); + expect(screen.getByRole('menuitemradio', { name: /Right/ })).toBeInTheDocument(); }); it('for far-bottom it should set the correct label active (by setting the secondary color)', () => { createWrapper({ thumbnailNavigationPosition: 'far-bottom' }); - expect(screen.getByRole('menuitem', { name: /Bottom/ }).querySelector('svg')).toHaveClass('MuiSvgIcon-colorSecondary'); // eslint-disable-line testing-library/no-node-access - expect(screen.getByRole('menuitem', { name: /Right/ }).querySelector('svg')).not.toHaveClass('MuiSvgIcon-colorSecondary'); // eslint-disable-line testing-library/no-node-access - expect(screen.getByRole('menuitem', { name: /Off/ }).querySelector('svg')).not.toHaveClass('MuiSvgIcon-colorSecondary'); // eslint-disable-line testing-library/no-node-access + expect(screen.getByRole('menuitemradio', { name: /Bottom/ }).querySelector('svg')).toHaveClass('MuiSvgIcon-colorSecondary'); // eslint-disable-line testing-library/no-node-access + expect(screen.getByRole('menuitemradio', { name: /Right/ }).querySelector('svg')).not.toHaveClass('MuiSvgIcon-colorSecondary'); // eslint-disable-line testing-library/no-node-access + expect(screen.getByRole('menuitemradio', { name: /Off/ }).querySelector('svg')).not.toHaveClass('MuiSvgIcon-colorSecondary'); // eslint-disable-line testing-library/no-node-access }); it('for far-right it should set the correct label active (by setting the secondary color)', () => { createWrapper({ thumbnailNavigationPosition: 'far-right' }); - expect(screen.getByRole('menuitem', { name: /Right/ }).querySelector('svg')).toHaveClass('MuiSvgIcon-colorSecondary'); // eslint-disable-line testing-library/no-node-access - expect(screen.getByRole('menuitem', { name: /Off/ }).querySelector('svg')).not.toHaveClass('MuiSvgIcon-colorSecondary'); // eslint-disable-line testing-library/no-node-access - expect(screen.getByRole('menuitem', { name: /Bottom/ }).querySelector('svg')).not.toHaveClass('MuiSvgIcon-colorSecondary'); // eslint-disable-line testing-library/no-node-access + expect(screen.getByRole('menuitemradio', { name: /Right/ }).querySelector('svg')).toHaveClass('MuiSvgIcon-colorSecondary'); // eslint-disable-line testing-library/no-node-access + expect(screen.getByRole('menuitemradio', { name: /Off/ }).querySelector('svg')).not.toHaveClass('MuiSvgIcon-colorSecondary'); // eslint-disable-line testing-library/no-node-access + expect(screen.getByRole('menuitemradio', { name: /Bottom/ }).querySelector('svg')).not.toHaveClass('MuiSvgIcon-colorSecondary'); // eslint-disable-line testing-library/no-node-access }); it('updates state when the thumbnail config selection changes', async () => { const setWindowThumbnailPosition = vi.fn(); const user = userEvent.setup(); createWrapper({ setWindowThumbnailPosition }); - const menuItems = screen.queryAllByRole('menuitem'); + const menuItems = screen.queryAllByRole('menuitemradio'); expect(menuItems.length).toBe(3); expect(menuItems[0]).toBeInTheDocument(); expect(menuItems[1]).toBeInTheDocument(); @@ -57,6 +57,6 @@ describe('WindowThumbnailSettings', () => { it('when rtl flips an icon', () => { createWrapper({ direction: 'rtl' }); - expect(screen.getByRole('menuitem', { name: /Right/ }).querySelector('svg')).toHaveStyle('transform: rotate(180deg);'); // eslint-disable-line testing-library/no-node-access + expect(screen.getByRole('menuitemradio', { name: /Right/ }).querySelector('svg')).toHaveStyle('transform: rotate(180deg);'); // eslint-disable-line testing-library/no-node-access }); }); diff --git a/__tests__/src/components/WindowTopMenu.test.js b/__tests__/src/components/WindowTopMenu.test.js index 6fd518c95..f023e06cf 100644 --- a/__tests__/src/components/WindowTopMenu.test.js +++ b/__tests__/src/components/WindowTopMenu.test.js @@ -33,16 +33,21 @@ describe('WindowTopMenu', () => { const menuSections = within(screen.getByRole('menu')).getAllByRole('presentation'); expect(menuSections).toHaveLength(2); + expect(menuSections[0]).toHaveTextContent('View'); - expect(menuSections[1]).toHaveTextContent('Thumbnails'); + const menus = within(screen.getByRole('menu')).getAllByRole('menubar'); - const menuItems = screen.getAllByRole('menuitem'); - expect(menuItems).toHaveLength(5); - expect(menuItems[0]).toHaveTextContent('Single'); - expect(menuItems[1]).toHaveTextContent('Gallery'); - expect(menuItems[2]).toHaveTextContent('Off'); - expect(menuItems[3]).toHaveTextContent('Bottom'); - expect(menuItems[4]).toHaveTextContent('Right'); + const viewItems = within(menus[0]).getAllByRole('menuitemradio'); + expect(viewItems).toHaveLength(2); + expect(viewItems[0]).toHaveTextContent('Single'); + expect(viewItems[1]).toHaveTextContent('Gallery'); + + expect(menuSections[1]).toHaveTextContent('Thumbnails'); + const thumbnailItems = within(menus[1]).getAllByRole('menuitemradio'); + expect(thumbnailItems).toHaveLength(3); + expect(thumbnailItems[0]).toHaveTextContent('Off'); + expect(thumbnailItems[1]).toHaveTextContent('Bottom'); + expect(thumbnailItems[2]).toHaveTextContent('Right'); }); it('does not display unless open', () => { @@ -66,7 +71,7 @@ describe('WindowTopMenu', () => { />); // click a menu item should close the menu - const menuItems = screen.getAllByRole('menuitem'); + const menuItems = screen.getAllByRole('menuitemradio'); await user.click(menuItems[0]); expect(handleClose).toHaveBeenCalledTimes(1); expect(toggleDraggingEnabled).toHaveBeenCalledTimes(1); diff --git a/__tests__/src/components/WindowTopMenuButton.test.js b/__tests__/src/components/WindowTopMenuButton.test.js index baf91aef5..a1c0ce9bb 100644 --- a/__tests__/src/components/WindowTopMenuButton.test.js +++ b/__tests__/src/components/WindowTopMenuButton.test.js @@ -33,7 +33,7 @@ describe('WindowTopMenuButton', () => { expect(screen.getByRole('menu')).toBeInTheDocument(); // click something else to close the menu (the windowMenu button is hidden at this point) - await user.click(screen.getAllByRole('menuitem')[0]); + await user.click(screen.getAllByRole('menuitemradio')[0]); expect(screen.queryByRole('menu')).not.toBeInTheDocument(); }); diff --git a/__tests__/src/components/WindowViewSettings.test.js b/__tests__/src/components/WindowViewSettings.test.js index ad8efe97e..80e0cc627 100644 --- a/__tests__/src/components/WindowViewSettings.test.js +++ b/__tests__/src/components/WindowViewSettings.test.js @@ -20,7 +20,7 @@ describe('WindowViewSettings', () => { it('renders all elements correctly', () => { createWrapper(); expect(screen.getByRole('presentation', { selector: 'li' })).toBeInTheDocument(); - const menuItems = screen.queryAllByRole('menuitem'); + const menuItems = screen.queryAllByRole('menuitemradio'); expect(menuItems.length).toBe(4); expect(menuItems[0]).toHaveTextContent(/Single/i); expect(menuItems[1]).toHaveTextContent(/Book/i); @@ -29,29 +29,29 @@ describe('WindowViewSettings', () => { }); it('single should set the correct label active (by setting the secondary color)', () => { createWrapper({ windowViewType: 'single' }); - expect(screen.getByRole('menuitem', { name: /Single/ }).querySelector('svg')).toHaveClass('MuiSvgIcon-colorSecondary'); // eslint-disable-line testing-library/no-node-access - expect(screen.getByRole('menuitem', { name: /Book/ }).querySelector('svg')).not.toHaveClass('MuiSvgIcon-colorSecondary'); // eslint-disable-line testing-library/no-node-access + expect(screen.getByRole('menuitemradio', { name: /Single/ }).querySelector('svg')).toHaveClass('MuiSvgIcon-colorSecondary'); // eslint-disable-line testing-library/no-node-access + expect(screen.getByRole('menuitemradio', { name: /Book/ }).querySelector('svg')).not.toHaveClass('MuiSvgIcon-colorSecondary'); // eslint-disable-line testing-library/no-node-access }); it('book should set the correct label active (by setting the secondary color)', () => { createWrapper({ windowViewType: 'book' }); - expect(screen.getByRole('menuitem', { name: /Book/ }).querySelector('svg')).toHaveClass('MuiSvgIcon-colorSecondary'); // eslint-disable-line testing-library/no-node-access - expect(screen.getByRole('menuitem', { name: /Single/ }).querySelector('svg')).not.toHaveClass('MuiSvgIcon-colorSecondary'); // eslint-disable-line testing-library/no-node-access + expect(screen.getByRole('menuitemradio', { name: /Book/ }).querySelector('svg')).toHaveClass('MuiSvgIcon-colorSecondary'); // eslint-disable-line testing-library/no-node-access + expect(screen.getByRole('menuitemradio', { name: /Single/ }).querySelector('svg')).not.toHaveClass('MuiSvgIcon-colorSecondary'); // eslint-disable-line testing-library/no-node-access }); it('scroll should set the correct label active (by setting the secondary color)', () => { createWrapper({ windowViewType: 'scroll' }); - expect(screen.getByRole('menuitem', { name: /Scroll/ }).querySelector('svg')).toHaveClass('MuiSvgIcon-colorSecondary'); // eslint-disable-line testing-library/no-node-access - expect(screen.getByRole('menuitem', { name: /Single/ }).querySelector('svg')).not.toHaveClass('MuiSvgIcon-colorSecondary'); // eslint-disable-line testing-library/no-node-access + expect(screen.getByRole('menuitemradio', { name: /Scroll/ }).querySelector('svg')).toHaveClass('MuiSvgIcon-colorSecondary'); // eslint-disable-line testing-library/no-node-access + expect(screen.getByRole('menuitemradio', { name: /Single/ }).querySelector('svg')).not.toHaveClass('MuiSvgIcon-colorSecondary'); // eslint-disable-line testing-library/no-node-access }); it('gallery should set the correct label active (by setting the secondary color)', () => { createWrapper({ windowViewType: 'gallery' }); - expect(screen.getByRole('menuitem', { name: /Gallery/ }).querySelector('svg')).toHaveClass('MuiSvgIcon-colorSecondary'); // eslint-disable-line testing-library/no-node-access - expect(screen.getByRole('menuitem', { name: /Single/ }).querySelector('svg')).not.toHaveClass('MuiSvgIcon-colorSecondary'); // eslint-disable-line testing-library/no-node-access + expect(screen.getByRole('menuitemradio', { name: /Gallery/ }).querySelector('svg')).toHaveClass('MuiSvgIcon-colorSecondary'); // eslint-disable-line testing-library/no-node-access + expect(screen.getByRole('menuitemradio', { name: /Single/ }).querySelector('svg')).not.toHaveClass('MuiSvgIcon-colorSecondary'); // eslint-disable-line testing-library/no-node-access }); it('updates state when the view config selection changes', async () => { const setWindowViewType = vi.fn(); createWrapper({ setWindowViewType }); const user = userEvent.setup(); - const menuItems = screen.queryAllByRole('menuitem'); + const menuItems = screen.queryAllByRole('menuitemradio'); expect(menuItems.length).toBe(4); await user.click(menuItems[0]); expect(setWindowViewType).toHaveBeenCalledWith('xyz', 'single'); diff --git a/src/components/WindowThumbnailSettings.js b/src/components/WindowThumbnailSettings.js index a5b94b20d..7fb3b8a3b 100644 --- a/src/components/WindowThumbnailSettings.js +++ b/src/components/WindowThumbnailSettings.js @@ -2,6 +2,7 @@ import { styled } from '@mui/material/styles'; import FormControlLabel from '@mui/material/FormControlLabel'; import ListSubheader from '@mui/material/ListSubheader'; import MenuItem from '@mui/material/MenuItem'; +import MenuList from '@mui/material/MenuList'; import ThumbnailsOffIcon from '@mui/icons-material/CropDinSharp'; import PropTypes from 'prop-types'; import { useTranslation } from 'react-i18next'; @@ -14,20 +15,23 @@ const ThumbnailOption = styled(MenuItem, { name: 'WindowThumbnailSettings', slot ...(selected && { borderBottomColor: theme.palette.secondary.main, }), + '&.Mui-selected': { + backgroundColor: 'transparent !important', + }, + '&.Mui-selected.Mui-focusVisible': { + backgroundColor: `${(theme.vars || theme).palette.action.focus} !important`, + }, + '&:focused': { + backgroundColor: `${(theme.vars || theme).palette.action.focus} !important`, + }, + color: selected ? theme.palette.secondary.main : undefined, + display: 'inline-flex', }, - '&.Mui-selected': { - backgroundColor: 'transparent !important', - }, - '&.Mui-selected.Mui-focusVisible': { - backgroundColor: `${(theme.vars || theme).palette.action.focus} !important`, - }, - '&:focused': { - backgroundColor: `${(theme.vars || theme).palette.action.focus} !important`, - }, - color: selected ? theme.palette.secondary.main : undefined, - display: 'inline-block', })); +const StyledMenuList = styled(MenuList, { name: 'WindowViewSettings', slot: 'option' })(() => ({ + display: 'inline-flex', +})); /** * */ @@ -40,43 +44,66 @@ export function WindowThumbnailSettings({ return ( <> - {t('thumbnails')} - - { handleChange('off'); }}> - - } - label={t('off')} - labelPlacement="bottom" - /> - - { handleChange('far-bottom'); }}> - - } - label={t('bottom')} - labelPlacement="bottom" - /> - - { handleChange('far-right'); }}> - - )} - label={t('right')} - labelPlacement="bottom" - /> - + {t('thumbnails')} + + { handleChange('off'); }} + role="menuitemradio" + selected={thumbnailNavigationPosition === 'off'} + > + + } + label={t('off')} + labelPlacement="bottom" + value="off" + /> + + { handleChange('far-bottom'); }} + role="menuitemradio" + selected={thumbnailNavigationPosition === 'far-bottom'} + > + + } + label={t('bottom')} + labelPlacement="bottom" + value="far-bottom" + /> + + { handleChange('far-right'); }} + role="menuitemradio" + selected={thumbnailNavigationPosition === 'far-right'} + > + + )} + label={t('right')} + labelPlacement="bottom" + value="far-right" + /> + + + ); } diff --git a/src/components/WindowTopMenu.js b/src/components/WindowTopMenu.js index a39e940e1..542ee66fa 100644 --- a/src/components/WindowTopMenu.js +++ b/src/components/WindowTopMenu.js @@ -1,6 +1,6 @@ import { useContext } from 'react'; -import Menu from '@mui/material/Menu'; import ListSubheader from '@mui/material/ListSubheader'; +import Popover from '@mui/material/Popover'; import PropTypes from 'prop-types'; import WindowThumbnailSettings from '../containers/WindowThumbnailSettings'; import WindowViewSettings from '../containers/WindowViewSettings'; @@ -28,7 +28,7 @@ export function WindowTopMenu({ const pluginProps = arguments[0]; // eslint-disable-line prefer-rest-params return ( - {showThumbnailNavigationSettings && } - + ); } diff --git a/src/components/WindowViewSettings.js b/src/components/WindowViewSettings.js index 76a0f480c..e9cebbafb 100644 --- a/src/components/WindowViewSettings.js +++ b/src/components/WindowViewSettings.js @@ -1,6 +1,7 @@ import { styled } from '@mui/material/styles'; import FormControlLabel from '@mui/material/FormControlLabel'; import MenuItem from '@mui/material/MenuItem'; +import MenuList from '@mui/material/MenuList'; import ListSubheader from '@mui/material/ListSubheader'; import SingleIcon from '@mui/icons-material/CropOriginalSharp'; import ScrollViewIcon from '@mui/icons-material/ViewColumn'; @@ -15,18 +16,22 @@ const ViewOption = styled(MenuItem, { name: 'WindowViewSettings', slot: 'option' ...(selected && { borderBottomColor: theme.palette.secondary.main, }), + '&.Mui-selected': { + backgroundColor: 'transparent !important', + }, + '&.Mui-selected.Mui-focusVisible': { + backgroundColor: `${(theme.vars || theme).palette.action.focus} !important`, + }, + '&:focused': { + backgroundColor: `${(theme.vars || theme).palette.action.focus} !important`, + }, + color: selected ? theme.palette.secondary.main : undefined, + display: 'inline-block', }, - '&.Mui-selected': { - backgroundColor: 'transparent !important', - }, - '&.Mui-selected.Mui-focusVisible': { - backgroundColor: `${(theme.vars || theme).palette.action.focus} !important`, - }, - '&:focused': { - backgroundColor: `${(theme.vars || theme).palette.action.focus} !important`, - }, - color: selected ? theme.palette.secondary.main : undefined, - display: 'inline-block', +})); + +const StyledMenuList = styled(MenuList, { name: 'WindowViewSettings', slot: 'option' })(() => ({ + display: 'inline-flex', })); /** @@ -52,10 +57,12 @@ export function WindowViewSettings({ none of the click handlers work? */ const menuItem = ({ value, Icon }) => ( { handleChange(value); handleClose(); }} + role="menuitemradio" + selected={windowViewType === value} > - {t('view')} - { viewTypes.map(value => menuItem({ Icon: iconMap[value], value })) } + {t('view')} + + { viewTypes.map(value => menuItem({ Icon: iconMap[value], value })) } + ); }