Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(a11y): wrong focus selection on toggleFullscreen method #1431

Merged
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion src/lib/viewers/BaseViewer.js
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,9 @@ class BaseViewer extends EventEmitter {
// From the perspective of viewers bp-content holds everything
this.containerEl = container.querySelector(SELECTOR_BOX_PREVIEW_CONTENT);

// Element to focus when fullscreen is toggled.
this.fullscreenToggleEl = null;
matiaslionel marked this conversation as resolved.
Show resolved Hide resolved

// Set an aria-label for all files
this.containerEl.setAttribute('aria-label', __('file_preview_label'));

Expand Down Expand Up @@ -551,9 +554,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) {
ConradJChan marked this conversation as resolved.
Show resolved Hide resolved
this.fullscreenToggleEl = fullscreenToggleEl;
ConradJChan marked this conversation as resolved.
Show resolved Hide resolved
fullscreen.toggle(this.containerEl);
}

Expand All @@ -569,6 +575,9 @@ class BaseViewer extends EventEmitter {
this.annotator.emit(ANNOTATOR_EVENT.setVisibility, false);
this.disableAnnotationControls();
}
if (this.fullscreenToggleEl && this.fullscreenToggleEl.focus) {
matiaslionel marked this conversation as resolved.
Show resolved Hide resolved
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, toggleFullscreen: EventTarget | null) => void;
matiaslionel marked this conversation as resolved.
Show resolved Hide resolved
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 = (event: React.MouseEvent<HTMLButtonElement, MouseEvent>): void => {
matiaslionel marked this conversation as resolved.
Show resolved Hide resolved
onFullscreenToggle(!isFullscreen, event.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