From a8153b708a33885e8dfbaaaa098993c45e9c7b7b Mon Sep 17 00:00:00 2001 From: Gildas Garcia <1122076+djhi@users.noreply.github.com> Date: Fri, 13 Sep 2024 17:54:45 +0200 Subject: [PATCH 1/2] Fix FilterButton accessibility and tests --- .../src/list/filter/FilterButton.spec.tsx | 58 +++++++++---------- .../src/list/filter/FilterButtonMenuItem.tsx | 2 + 2 files changed, 31 insertions(+), 29 deletions(-) diff --git a/packages/ra-ui-materialui/src/list/filter/FilterButton.spec.tsx b/packages/ra-ui-materialui/src/list/filter/FilterButton.spec.tsx index 63a74aea7e8..bfceb9e8ca0 100644 --- a/packages/ra-ui-materialui/src/list/filter/FilterButton.spec.tsx +++ b/packages/ra-ui-materialui/src/list/filter/FilterButton.spec.tsx @@ -49,26 +49,27 @@ describe('', () => { fireEvent.click(await screen.findByLabelText('Add filter')); - let checkboxs: HTMLInputElement[] = screen.getAllByRole('checkbox'); - expect(checkboxs).toHaveLength(3); - expect(checkboxs[0].checked).toBe(false); - expect(checkboxs[1].checked).toBe(false); - expect(checkboxs[2].checked).toBe(false); + let checkboxes: HTMLInputElement[] = + screen.getAllByRole('menuitemcheckbox'); + expect(checkboxes).toHaveLength(3); + expect(checkboxes[0].getAttribute('aria-checked')).toBe('false'); + expect(checkboxes[1].getAttribute('aria-checked')).toBe('false'); + expect(checkboxes[2].getAttribute('aria-checked')).toBe('false'); - fireEvent.click(checkboxs[0]); + fireEvent.click(checkboxes[0]); await screen.findByRole('textbox', { name: 'Title', }); fireEvent.click(screen.getByLabelText('Add filter')); - checkboxs = screen.getAllByRole('checkbox'); - expect(checkboxs).toHaveLength(3); - expect(checkboxs[0].checked).toBe(true); - expect(checkboxs[1].checked).toBe(false); - expect(checkboxs[2].checked).toBe(false); + checkboxes = screen.getAllByRole('menuitemcheckbox'); + expect(checkboxes).toHaveLength(3); + expect(checkboxes[0].getAttribute('aria-checked')).toBe('true'); + expect(checkboxes[1].getAttribute('aria-checked')).toBe('false'); + expect(checkboxes[2].getAttribute('aria-checked')).toBe('false'); - fireEvent.click(checkboxs[0]); + fireEvent.click(checkboxes[0]); await waitFor( () => { @@ -82,11 +83,11 @@ describe('', () => { ); fireEvent.click(screen.getByLabelText('Add filter')); - checkboxs = screen.getAllByRole('checkbox'); - expect(checkboxs).toHaveLength(3); - expect(checkboxs[0].checked).toBe(false); - expect(checkboxs[1].checked).toBe(false); - expect(checkboxs[2].checked).toBe(false); + checkboxes = screen.getAllByRole('menuitemcheckbox'); + expect(checkboxes).toHaveLength(3); + expect(checkboxes[0].getAttribute('aria-checked')).toBe('false'); + expect(checkboxes[1].getAttribute('aria-checked')).toBe('false'); + expect(checkboxes[2].getAttribute('aria-checked')).toBe('false'); }, 7000); it('should remove the checked state of the menu item when removing its matching filter', async () => { @@ -94,8 +95,9 @@ describe('', () => { fireEvent.click(await screen.findByLabelText('Add filter')); - let checkboxs: HTMLInputElement[] = screen.getAllByRole('checkbox'); - fireEvent.click(checkboxs[0]); + let checkboxes: HTMLInputElement[] = + screen.getAllByRole('menuitemcheckbox'); + fireEvent.click(checkboxes[0]); await screen.findByRole('textbox', { name: 'Title', @@ -115,11 +117,11 @@ describe('', () => { ); fireEvent.click(screen.getByLabelText('Add filter')); - checkboxs = screen.getAllByRole('checkbox'); - expect(checkboxs).toHaveLength(3); - expect(checkboxs[0].checked).toBe(false); - expect(checkboxs[1].checked).toBe(false); - expect(checkboxs[2].checked).toBe(false); + checkboxes = screen.getAllByRole('menuitemcheckbox'); + expect(checkboxes).toHaveLength(3); + expect(checkboxes[0].getAttribute('aria-checked')).toBe('false'); + expect(checkboxes[1].getAttribute('aria-checked')).toBe('false'); + expect(checkboxes[2].getAttribute('aria-checked')).toBe('false'); }); it('should display the filter button if all filters are shown and there is a filter value', () => { @@ -191,13 +193,11 @@ describe('', () => { await screen.findByText('Add filter'); fireEvent.click(screen.getByText('Add filter')); - await screen.findByText('Title', { selector: 'li > span' }); + await screen.findByText('Title', { selector: 'li span' }); expect(screen.queryByDisplayValue('Remove all filters')).toBeNull(); // Then we apply a filter - fireEvent.click( - screen.getByText('Title', { selector: 'li > span' }) - ); + fireEvent.click(screen.getByText('Title', { selector: 'li span' })); await screen.findByDisplayValue( 'Accusantium qui nihil voluptatum quia voluptas maxime ab similique' ); @@ -224,7 +224,7 @@ describe('', () => { await screen.findByText('Add filter'); fireEvent.click(screen.getByText('Add filter')); - await screen.findByText('Title', { selector: 'li > span' }); + await screen.findByText('Title', { selector: 'li span' }); expect(screen.queryByDisplayValue('Remove all filters')).toBeNull(); // Then we apply a filter to an alwaysOn filter diff --git a/packages/ra-ui-materialui/src/list/filter/FilterButtonMenuItem.tsx b/packages/ra-ui-materialui/src/list/filter/FilterButtonMenuItem.tsx index 49d613624d9..2a3ff3bb887 100644 --- a/packages/ra-ui-materialui/src/list/filter/FilterButtonMenuItem.tsx +++ b/packages/ra-ui-materialui/src/list/filter/FilterButtonMenuItem.tsx @@ -31,6 +31,8 @@ export const FilterButtonMenuItem = forwardRef( autoFocus={autoFocus} ref={ref} disabled={filter.props.disabled} + role="menuitemcheckbox" + aria-checked={displayed} > {displayed ? ( From 947fd6dfda8299522b1ac37c9f28b62297c3e539 Mon Sep 17 00:00:00 2001 From: Gildas Garcia <1122076+djhi@users.noreply.github.com> Date: Fri, 13 Sep 2024 18:03:11 +0200 Subject: [PATCH 2/2] Fix FilterForm tests --- packages/ra-ui-materialui/src/list/filter/FilterForm.spec.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/ra-ui-materialui/src/list/filter/FilterForm.spec.tsx b/packages/ra-ui-materialui/src/list/filter/FilterForm.spec.tsx index 7f9cdb4b352..e13e0011332 100644 --- a/packages/ra-ui-materialui/src/list/filter/FilterForm.spec.tsx +++ b/packages/ra-ui-materialui/src/list/filter/FilterForm.spec.tsx @@ -376,7 +376,7 @@ describe('', () => { // Set nested filter value to 'bar' fireEvent.click(await screen.findByLabelText('Add filter')); fireEvent.click( - await screen.findByRole('menuitem', { name: 'Nested' }) + await screen.findByRole('menuitemcheckbox', { name: 'Nested' }) ); fireEvent.click(await screen.findByText('bar')); fireEvent.blur(await screen.findByLabelText('Nested'));