From 14cf6275d04758c2fa343acc351d13d7d5bc0919 Mon Sep 17 00:00:00 2001 From: Kerry Date: Fri, 27 May 2022 09:13:50 +0200 Subject: [PATCH] Fix: AccessibleButton does not set disabled attribute (PSF-1055) (#8682) * remove old styles for pin drop buttons Signed-off-by: Kerry Archibald * fully disable share location button until location is shared Signed-off-by: Kerry Archibald * set disabled on button Signed-off-by: Kerry Archibald * test AccessibleButton disabled Signed-off-by: Kerry Archibald * remove disbaled check in LocationPicker Signed-off-by: Kerry Archibald --- .../components/views/location/_ShareType.scss | 8 - .../views/elements/AccessibleButton.tsx | 1 + .../views/elements/AccessibleButton-test.tsx | 204 ++++++++++++++++++ .../AccessibleButton-test.tsx.snap | 62 ++++++ .../PollCreateDialog-test.tsx.snap | 2 +- .../views/location/LocationPicker-test.tsx | 22 ++ .../LocationShareMenu-test.tsx.snap | 1 + 7 files changed, 291 insertions(+), 9 deletions(-) create mode 100644 test/components/views/elements/AccessibleButton-test.tsx create mode 100644 test/components/views/elements/__snapshots__/AccessibleButton-test.tsx.snap diff --git a/res/css/components/views/location/_ShareType.scss b/res/css/components/views/location/_ShareType.scss index 458be106eb..6b39f1a80a 100644 --- a/res/css/components/views/location/_ShareType.scss +++ b/res/css/components/views/location/_ShareType.scss @@ -63,14 +63,6 @@ limitations under the License. &:hover, &:focus { border-color: $accent; } - - // this style is only during active development - // when lab is enabled but feature not fully implemented - // pin drop option will be disabled - &.mx_AccessibleButton_disabled { - pointer-events: none; - opacity: 0.4; - } } .mx_ShareType_option-icon { diff --git a/src/components/views/elements/AccessibleButton.tsx b/src/components/views/elements/AccessibleButton.tsx index 36600cbfb3..753ee722bd 100644 --- a/src/components/views/elements/AccessibleButton.tsx +++ b/src/components/views/elements/AccessibleButton.tsx @@ -85,6 +85,7 @@ export default function AccessibleButton({ const newProps: IAccessibleButtonProps = restProps; if (disabled) { newProps["aria-disabled"] = true; + newProps["disabled"] = true; } else { if (triggerOnMouseDown) { newProps.onMouseDown = onClick; diff --git a/test/components/views/elements/AccessibleButton-test.tsx b/test/components/views/elements/AccessibleButton-test.tsx new file mode 100644 index 0000000000..89385dd057 --- /dev/null +++ b/test/components/views/elements/AccessibleButton-test.tsx @@ -0,0 +1,204 @@ +/* +Copyright 2022 The Matrix.org Foundation C.I.C. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +import React from 'react'; +import { mount } from 'enzyme'; +import { act } from 'react-dom/test-utils'; + +import AccessibleButton from '../../../../src/components/views/elements/AccessibleButton'; +import { Key } from '../../../../src/Keyboard'; +import { mockPlatformPeg, unmockPlatformPeg } from '../../../test-utils'; + +describe('', () => { + const defaultProps = { + onClick: jest.fn(), + children: 'i am a button', + }; + const getComponent = (props = {}) => + mount(); + + beforeEach(() => { + mockPlatformPeg(); + }); + + afterAll(() => { + unmockPlatformPeg(); + }); + + const makeKeyboardEvent = (key: string) => ({ + key, + stopPropagation: jest.fn(), + preventDefault: jest.fn(), + }) as unknown as KeyboardEvent; + + it('renders div with role button by default', () => { + const component = getComponent(); + expect(component).toMatchSnapshot(); + }); + + it('renders a button element', () => { + const component = getComponent({ element: 'button' }); + expect(component).toMatchSnapshot(); + }); + + it('renders with correct classes when button has kind', () => { + const component = getComponent({ + kind: 'primary', + }); + expect(component).toMatchSnapshot(); + }); + + it('disables button correctly', () => { + const onClick = jest.fn(); + const component = getComponent({ + onClick, + disabled: true, + }); + expect(component.find('.mx_AccessibleButton').props().disabled).toBeTruthy(); + expect(component.find('.mx_AccessibleButton').props()['aria-disabled']).toBeTruthy(); + + act(() => { + component.simulate('click'); + }); + + expect(onClick).not.toHaveBeenCalled(); + + act(() => { + const keydownEvent = makeKeyboardEvent(Key.ENTER); + component.simulate('keydown', keydownEvent); + }); + + expect(onClick).not.toHaveBeenCalled(); + }); + + it('calls onClick handler on button click', () => { + const onClick = jest.fn(); + const component = getComponent({ + onClick, + }); + + act(() => { + component.simulate('click'); + }); + + expect(onClick).toHaveBeenCalled(); + }); + + it('calls onClick handler on button mousedown when triggerOnMousedown is passed', () => { + const onClick = jest.fn(); + const component = getComponent({ + onClick, + triggerOnMouseDown: true, + }); + + act(() => { + component.simulate('mousedown'); + }); + + expect(onClick).toHaveBeenCalled(); + }); + + describe('handling keyboard events', () => { + it('calls onClick handler on enter keydown', () => { + const onClick = jest.fn(); + const component = getComponent({ + onClick, + }); + + const keyboardEvent = makeKeyboardEvent(Key.ENTER); + act(() => { + component.simulate('keydown', keyboardEvent); + }); + + expect(onClick).toHaveBeenCalled(); + + act(() => { + component.simulate('keyup', keyboardEvent); + }); + + // handler only called once on keydown + expect(onClick).toHaveBeenCalledTimes(1); + // called for both keyup and keydown + expect(keyboardEvent.stopPropagation).toHaveBeenCalledTimes(2); + expect(keyboardEvent.preventDefault).toHaveBeenCalledTimes(2); + }); + + it('calls onClick handler on space keyup', () => { + const onClick = jest.fn(); + const component = getComponent({ + onClick, + }); + + const keyboardEvent = makeKeyboardEvent(Key.SPACE); + act(() => { + component.simulate('keydown', keyboardEvent); + }); + + expect(onClick).not.toHaveBeenCalled(); + + act(() => { + component.simulate('keyup', keyboardEvent); + }); + + // handler only called once on keyup + expect(onClick).toHaveBeenCalledTimes(1); + // called for both keyup and keydown + expect(keyboardEvent.stopPropagation).toHaveBeenCalledTimes(2); + expect(keyboardEvent.preventDefault).toHaveBeenCalledTimes(2); + }); + + it('calls onKeydown/onKeyUp handlers for keys other than space and enter', () => { + const onClick = jest.fn(); + const onKeyDown = jest.fn(); + const onKeyUp = jest.fn(); + const component = getComponent({ + onClick, + onKeyDown, + onKeyUp, + }); + + const keyboardEvent = makeKeyboardEvent(Key.K); + act(() => { + component.simulate('keydown', keyboardEvent); + component.simulate('keyup', keyboardEvent); + }); + + expect(onClick).not.toHaveBeenCalled(); + expect(onKeyDown).toHaveBeenCalled(); + expect(onKeyUp).toHaveBeenCalled(); + expect(keyboardEvent.stopPropagation).not.toHaveBeenCalled(); + expect(keyboardEvent.preventDefault).not.toHaveBeenCalled(); + }); + + it('does nothing on non space/enter key presses when no onKeydown/onKeyUp handlers provided', () => { + const onClick = jest.fn(); + const component = getComponent({ + onClick, + }); + + const keyboardEvent = makeKeyboardEvent(Key.K); + act(() => { + component.simulate('keydown', keyboardEvent); + component.simulate('keyup', keyboardEvent); + }); + + // no onClick call, no problems + expect(onClick).not.toHaveBeenCalled(); + expect(keyboardEvent.stopPropagation).not.toHaveBeenCalled(); + expect(keyboardEvent.preventDefault).not.toHaveBeenCalled(); + }); + }); +}); diff --git a/test/components/views/elements/__snapshots__/AccessibleButton-test.tsx.snap b/test/components/views/elements/__snapshots__/AccessibleButton-test.tsx.snap new file mode 100644 index 0000000000..0d049b34f7 --- /dev/null +++ b/test/components/views/elements/__snapshots__/AccessibleButton-test.tsx.snap @@ -0,0 +1,62 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[` renders a button element 1`] = ` + + + +`; + +exports[` renders div with role button by default 1`] = ` + +
+ i am a button +
+ +`; + +exports[` renders with correct classes when button has kind 1`] = ` + +
+ i am a button +
+ +`; diff --git a/test/components/views/elements/__snapshots__/PollCreateDialog-test.tsx.snap b/test/components/views/elements/__snapshots__/PollCreateDialog-test.tsx.snap index 77e40f05e7..72c7fcc811 100644 --- a/test/components/views/elements/__snapshots__/PollCreateDialog-test.tsx.snap +++ b/test/components/views/elements/__snapshots__/PollCreateDialog-test.tsx.snap @@ -1,6 +1,6 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`PollCreateDialog renders a blank poll 1`] = `"

Create poll

Poll type

Voters see results as soon as they have voted

What is your poll question or topic?

Create options

Add option
Cancel
"`; +exports[`PollCreateDialog renders a blank poll 1`] = `"

Create poll

Poll type

Voters see results as soon as they have voted

What is your poll question or topic?

Create options

Add option
Cancel
"`; exports[`PollCreateDialog renders a question and some options 1`] = `"

Create poll

Poll type

Voters see results as soon as they have voted

What is your poll question or topic?

Create options

Add option
Cancel
"`; diff --git a/test/components/views/location/LocationPicker-test.tsx b/test/components/views/location/LocationPicker-test.tsx index 6dd25ae72d..bb2f8e6348 100644 --- a/test/components/views/location/LocationPicker-test.tsx +++ b/test/components/views/location/LocationPicker-test.tsx @@ -186,6 +186,28 @@ describe("LocationPicker", () => { expect(wrapper.find('MemberAvatar').length).toBeTruthy(); }); + it('disables submit button until geolocation completes', () => { + const onChoose = jest.fn(); + const wrapper = getComponent({ shareType, onChoose }); + + // submit button is enabled when position is truthy + expect(findByTestId(wrapper, 'location-picker-submit-button').at(0).props().disabled).toBeTruthy(); + act(() => { + findByTestId(wrapper, 'location-picker-submit-button').at(0).simulate('click'); + }); + // nothing happens on button click + expect(onChoose).not.toHaveBeenCalled(); + + act(() => { + // @ts-ignore + mocked(mockGeolocate).emit('geolocate', mockGeolocationPosition); + wrapper.setProps({}); + }); + + // submit button is enabled when position is truthy + expect(findByTestId(wrapper, 'location-picker-submit-button').at(0).props().disabled).toBeFalsy(); + }); + it('submits location', () => { const onChoose = jest.fn(); const wrapper = getComponent({ onChoose, shareType }); diff --git a/test/components/views/location/__snapshots__/LocationShareMenu-test.tsx.snap b/test/components/views/location/__snapshots__/LocationShareMenu-test.tsx.snap index 0de312367a..76f50889c0 100644 --- a/test/components/views/location/__snapshots__/LocationShareMenu-test.tsx.snap +++ b/test/components/views/location/__snapshots__/LocationShareMenu-test.tsx.snap @@ -89,6 +89,7 @@ exports[` with live location disabled goes to labs flag scr aria-disabled={true} className="mx_AccessibleButton mx_EnableLiveShare_button mx_AccessibleButton_hasKind mx_AccessibleButton_kind_primary mx_AccessibleButton_disabled" data-test-id="enable-live-share-submit" + disabled={true} role="button" tabIndex={0} >