From 5859dc348e5a6937ebdda2d2c022028ac68c9375 Mon Sep 17 00:00:00 2001 From: Lingzhi Xu Date: Tue, 19 Sep 2023 17:31:02 -0700 Subject: [PATCH] address comments and fix tests --- .../TablePagination/TablePagination.test.js | 22 +++++++++-------- .../mui-material/src/Select/Select.test.js | 2 +- .../mui-material/src/Select/SelectInput.js | 4 ++-- .../TablePagination/TablePagination.test.js | 24 ++++++++++--------- .../src/TextField/TextField.test.js | 4 ++-- 5 files changed, 30 insertions(+), 26 deletions(-) diff --git a/packages/mui-material-next/src/TablePagination/TablePagination.test.js b/packages/mui-material-next/src/TablePagination/TablePagination.test.js index e3052bb9bbe3ab..9c2e4ac19e36bb 100644 --- a/packages/mui-material-next/src/TablePagination/TablePagination.test.js +++ b/packages/mui-material-next/src/TablePagination/TablePagination.test.js @@ -101,8 +101,8 @@ describe('', () => { ); // will be `getByRole('combobox')` in aria 1.2 - const [combobox] = getAllByRole('button'); - expect(combobox).toHaveAccessibleName('lines per page: 10'); + const [combobox] = getAllByRole('combobox'); + expect(combobox).toHaveAccessibleName('lines per page:'); }); it('accepts React nodes', () => { @@ -128,14 +128,14 @@ describe('', () => { ); // will be `getByRole('combobox')` in aria 1.2 - const [combobox] = getAllByRole('button'); - expect(combobox).toHaveAccessibleName('lines per page: 10'); + const [combobox] = getAllByRole('combobox'); + expect(combobox).toHaveAccessibleName('lines per page:'); }); }); describe('prop: page', () => { it('should disable the back button on the first page', () => { - const { getAllByRole } = render( + const { getByRole } = render( @@ -151,13 +151,14 @@ describe('', () => {
, ); - const [, backButton, nextButton] = getAllByRole('button'); + const backButton = getByRole('button', { name: 'Go to previous page' }); + const nextButton = getByRole('button', { name: 'Go to next page' }); expect(backButton).to.have.property('disabled', true); expect(nextButton).to.have.property('disabled', false); }); it('should disable the next button on the last page', () => { - const { getAllByRole } = render( + const { getByRole } = render( @@ -173,7 +174,8 @@ describe('', () => {
, ); - const [, backButton, nextButton] = getAllByRole('button'); + const backButton = getByRole('button', { name: 'Go to previous page' }); + const nextButton = getByRole('button', { name: 'Go to next page' }); expect(backButton).to.have.property('disabled', false); expect(nextButton).to.have.property('disabled', true); }); @@ -398,8 +400,8 @@ describe('', () => { ); // will be `getByRole('combobox')` in aria 1.2 - const [combobox] = getAllByRole('button'); - expect(combobox).toHaveAccessibleName('Rows per page: 10'); + const [combobox] = getAllByRole('combobox'); + expect(combobox).toHaveAccessibleName('Rows per page:'); }); }); diff --git a/packages/mui-material/src/Select/Select.test.js b/packages/mui-material/src/Select/Select.test.js index f48e9218a84686..c861a904ac5055 100644 --- a/packages/mui-material/src/Select/Select.test.js +++ b/packages/mui-material/src/Select/Select.test.js @@ -482,7 +482,7 @@ describe('); const listboxId = getByRole('listbox').id; - expect(getByRole('combobox')).to.have.attribute('aria-controls', listboxId); + expect(getByRole('combobox', { hidden: true })).to.have.attribute('aria-controls', listboxId); }); specify('the listbox is focusable', () => { diff --git a/packages/mui-material/src/Select/SelectInput.js b/packages/mui-material/src/Select/SelectInput.js index 91e56162fc1b58..de66ec05f1ff68 100644 --- a/packages/mui-material/src/Select/SelectInput.js +++ b/packages/mui-material/src/Select/SelectInput.js @@ -5,7 +5,7 @@ import PropTypes from 'prop-types'; import clsx from 'clsx'; import MuiError from '@mui/utils/macros/MuiError.macro'; import { unstable_composeClasses as composeClasses } from '@mui/base/composeClasses'; -import { refType } from '@mui/utils'; +import { refType, unstable_useId as useId } from '@mui/utils'; import ownerDocument from '../utils/ownerDocument'; import capitalize from '../utils/capitalize'; import Menu from '../Menu/Menu'; @@ -487,7 +487,7 @@ const SelectInput = React.forwardRef(function SelectInput(props, ref) { ...MenuProps.slotProps?.paper, }; - const listboxId = React.useId(); + const listboxId = useId(); return ( diff --git a/packages/mui-material/src/TablePagination/TablePagination.test.js b/packages/mui-material/src/TablePagination/TablePagination.test.js index 53a3f7969ce6b4..2f9eed9e158a14 100644 --- a/packages/mui-material/src/TablePagination/TablePagination.test.js +++ b/packages/mui-material/src/TablePagination/TablePagination.test.js @@ -102,8 +102,8 @@ describe('', () => { ); // will be `getByRole('combobox')` in aria 1.2 - const [combobox] = getAllByRole('button'); - expect(combobox).toHaveAccessibleName('lines per page: 10'); + const [combobox] = getAllByRole('combobox'); + expect(combobox).toHaveAccessibleName('lines per page:'); }); it('accepts React nodes', () => { @@ -129,14 +129,14 @@ describe('', () => { ); // will be `getByRole('combobox')` in aria 1.2 - const [combobox] = getAllByRole('button'); - expect(combobox).toHaveAccessibleName('lines per page: 10'); + const [combobox] = getAllByRole('combobox'); + expect(combobox).toHaveAccessibleName('lines per page:'); }); }); describe('prop: page', () => { it('should disable the back button on the first page', () => { - const { getAllByRole } = render( + const { getByRole } = render( @@ -152,13 +152,14 @@ describe('', () => {
, ); - const [, backButton, nextButton] = getAllByRole('button'); + const backButton = getByRole('button', { name: 'Go to previous page' }); + const nextButton = getByRole('button', { name: 'Go to next page' }); expect(backButton).to.have.property('disabled', true); expect(nextButton).to.have.property('disabled', false); }); it('should disable the next button on the last page', () => { - const { getAllByRole } = render( + const { getByRole } = render( @@ -174,7 +175,8 @@ describe('', () => {
, ); - const [, backButton, nextButton] = getAllByRole('button'); + const backButton = getByRole('button', { name: 'Go to previous page' }); + const nextButton = getByRole('button', { name: 'Go to next page' }); expect(backButton).to.have.property('disabled', false); expect(nextButton).to.have.property('disabled', true); }); @@ -399,8 +401,8 @@ describe('', () => { ); // will be `getByRole('combobox')` in aria 1.2 - const [combobox] = getAllByRole('button'); - expect(combobox).toHaveAccessibleName('Rows per page: 10'); + const [combobox] = getAllByRole('combobox'); + expect(combobox).toHaveAccessibleName('Rows per page:'); }); ['standard', 'outlined', 'filled'].forEach((variant) => { @@ -422,7 +424,7 @@ describe('', () => { , ); - const [combobox] = getAllByRole('button'); + const [combobox] = getAllByRole('combobox'); const comboboxContainer = combobox.parentElement; if (variant === 'standard') { diff --git a/packages/mui-material/src/TextField/TextField.test.js b/packages/mui-material/src/TextField/TextField.test.js index b0910a0b22bbdb..ab60aabe55cd16 100644 --- a/packages/mui-material/src/TextField/TextField.test.js +++ b/packages/mui-material/src/TextField/TextField.test.js @@ -202,7 +202,7 @@ describe('', () => { , ); - expect(getByRole('button')).toHaveAccessibleName('Release: Stable'); + expect(getByRole('combobox')).toHaveAccessibleName('Release:'); }); it('creates an input[hidden] that has no accessible properties', () => { @@ -224,7 +224,7 @@ describe('', () => { , ); - expect(getByRole('button')).toHaveAccessibleDescription('Foo bar'); + expect(getByRole('combobox')).toHaveAccessibleDescription('Foo bar'); }); });