Skip to content

Commit

Permalink
fix(a11y): wrong focus selection on toggleFullscreen method (#1431)
Browse files Browse the repository at this point in the history
* fix(fullscreen): focus button when toggle fullscreen

* fix(fullscreen): add void to function

* fix(fullscreen): fix test

* fix(fullscreen): update names of methods and functions

* fix(fullscreen): destructure property

* fix(fullscreen): remove property

* fix(fullscreen): add property to class

* fix(fullscreen): clean property in destroy method

* fix(fullscreen): change method name for consistency
  • Loading branch information
matiaslionel authored Sep 24, 2021
1 parent 0d5a136 commit 8153de6
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 6 deletions.
12 changes: 11 additions & 1 deletion src/lib/viewers/BaseViewer.js
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,9 @@ class BaseViewer extends EventEmitter {
/** @property {HTMLElement} - The .bp-content which is the container for the viewer's content */
containerEl;

/** @property {HTMLButtonElement} - The button which will be focus when fullscreen is toggle */
fullscreenToggleEl;

/** @property {boolean} - Stores whether the Viewer has been setup yet. */
isSetup = false;

Expand Down Expand Up @@ -278,6 +281,7 @@ class BaseViewer extends EventEmitter {
this.annotatorPromise = null;
this.annotatorPromiseResolver = null;
this.emittedMetrics = null;
this.fullscreenToggleEl = null;
this.emit('destroy');
}

Expand Down Expand Up @@ -551,9 +555,12 @@ class BaseViewer extends EventEmitter {
* Enters or exits fullscreen
*
* @protected
* @param {boolean} [isFullscreen] - flag to allow fullscreen
* @param {HTMLElement} element - Element to be focused after fullscreen toggle
* @return {void}
*/
toggleFullscreen() {
toggleFullscreen(isFullscreen, fullscreenToggleEl) {
this.fullscreenToggleEl = fullscreenToggleEl;
fullscreen.toggle(this.containerEl);
}

Expand All @@ -569,6 +576,9 @@ class BaseViewer extends EventEmitter {
this.annotator.emit(ANNOTATOR_EVENT.setVisibility, false);
this.disableAnnotationControls();
}
if (this.fullscreenToggleEl && this.fullscreenToggleEl.focus) {
this.fullscreenToggleEl.focus();
}
}

/**
Expand Down
6 changes: 3 additions & 3 deletions src/lib/viewers/controls/fullscreen/FullscreenToggle.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import useFullscreen from '../hooks/useFullscreen';
import './FullscreenToggle.scss';

export type Props = {
onFullscreenToggle: (isFullscreen: boolean) => void;
onFullscreenToggle: (isFullscreen: boolean, toggleFullscreenEl: EventTarget | null) => void;
onKeyDown?: (event: React.KeyboardEvent<HTMLButtonElement>) => void;
};

Expand All @@ -14,8 +14,8 @@ export default function FullscreenToggle({ onFullscreenToggle, ...rest }: Props)
const Icon = isFullscreen ? IconFullscreenOut24 : IconFullscreenIn24;
const title = isFullscreen ? __('exit_fullscreen') : __('enter_fullscreen');

const handleClick = (): void => {
onFullscreenToggle(!isFullscreen);
const handleClick = ({ target }: React.MouseEvent): void => {
onFullscreenToggle(!isFullscreen, target);
};

return (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,11 @@ describe('FullscreenToggle', () => {

test('should invoke onFullscreenToggle prop on click', () => {
const onToggle = jest.fn();
const mockedEvent = { target: document.createElement('button') };
const wrapper = getWrapper({ onFullscreenToggle: onToggle });

wrapper.simulate('click');
expect(onToggle).toBeCalledWith(true);
wrapper.simulate('click', mockedEvent);
expect(onToggle).toBeCalledWith(true, mockedEvent.target);
});
});

Expand Down

0 comments on commit 8153de6

Please sign in to comment.