Skip to content

Commit

Permalink
Use ModalDialog in the notebook
Browse files Browse the repository at this point in the history
  • Loading branch information
acelaya committed Feb 2, 2024
1 parent 60bc609 commit e400d6c
Show file tree
Hide file tree
Showing 4 changed files with 67 additions and 19 deletions.
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
"@babel/preset-react": "^7.0.0",
"@babel/preset-typescript": "^7.16.7",
"@hypothesis/frontend-build": "^3.0.0",
"@hypothesis/frontend-shared": "^7.2.0",
"@hypothesis/frontend-shared": "^7.3.0",
"@hypothesis/frontend-testing": "^1.2.0",
"@npmcli/arborist": "^7.0.0",
"@octokit/rest": "^20.0.1",
Expand Down
51 changes: 39 additions & 12 deletions src/annotator/components/NotebookModal.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
import { IconButton, CancelIcon } from '@hypothesis/frontend-shared';
import {
IconButton,
CancelIcon,
ModalDialog,
} from '@hypothesis/frontend-shared';
import classnames from 'classnames';
import { useEffect, useRef, useState } from 'preact/hooks';
import type { Ref } from 'preact';
import { useEffect, useLayoutEffect, useRef, useState } from 'preact/hooks';

import { addConfigFragment } from '../../shared/config-fragment';
import { createAppConfig } from '../config/app';
Expand All @@ -19,15 +24,16 @@ export type NotebookConfig = {
type NotebookIframeProps = {
config: NotebookConfig;
groupId: string;
iframeRef: Ref<HTMLIFrameElement>;
};
/**
* Create the iframe that will load the notebook application.
*/
function NotebookIframe({ config, groupId }: NotebookIframeProps) {
function NotebookIframe({ config, groupId, iframeRef }: NotebookIframeProps) {
const notebookAppSrc = addConfigFragment(config.notebookAppUrl, {
...createAppConfig(config.notebookAppUrl, config),

// Explicity set the "focused" group
// Explicitly set the "focused" group
group: groupId,
});

Expand All @@ -37,9 +43,11 @@ function NotebookIframe({ config, groupId }: NotebookIframeProps) {
className="h-full w-full border-0"
allow="fullscreen; clipboard-write"
src={notebookAppSrc}
ref={iframeRef}
/>
);
}

export type NotebookModalProps = {
eventBus: EventBus;
config: NotebookConfig;
Expand All @@ -61,6 +69,7 @@ export default function NotebookModal({
const [groupId, setGroupId] = useState<string | null>(null);
const originalDocumentOverflowStyle = useRef('');
const emitterRef = useRef<Emitter | null>(null);
const iframeRef = useRef<HTMLIFrameElement | null>(null);

// Stores the original overflow CSS property of document.body and reset it
// when the component is destroyed
Expand Down Expand Up @@ -96,6 +105,17 @@ export default function NotebookModal({
};
}, [eventBus]);

useLayoutEffect(() => {
if (!isHidden) {
// When the notebook is shown, focus the iframe so that the next element
// in the tab sequence is an element inside of it.
// We can't use ModalDialog's initialFocus prop because it assumes the
// modal is destroyed when closed, and would cause the iframe to focus
// only the first time it's opened.
iframeRef.current?.focus();
}
}, [isHidden]);

const onClose = () => {
setIsHidden(true);
emitterRef.current?.publish('closeNotebook');
Expand All @@ -107,14 +127,16 @@ export default function NotebookModal({

return (
<div
className={classnames(
'fixed z-max top-0 left-0 right-0 bottom-0 p-3 bg-black/50',
{ hidden: isHidden },
)}
className={classnames({ hidden: isHidden })}
data-testid="notebook-outer"
>
<div className="relative w-full h-full" data-testid="notebook-inner">
<div className="absolute right-0 m-3">
<ModalDialog
variant="custom"
size="custom"
classes="p-3 relative w-full h-full"
initialFocus="manual"
>
<div className="absolute right-6 top-6">
<IconButton
title="Close the Notebook"
onClick={onClose}
Expand All @@ -130,8 +152,13 @@ export default function NotebookModal({
<CancelIcon className="w-4 h-4" />
</IconButton>
</div>
<NotebookIframe key={iframeKey} config={config} groupId={groupId} />
</div>
<NotebookIframe
key={iframeKey}
config={config}
groupId={groupId}
iframeRef={iframeRef}
/>
</ModalDialog>
</div>
);
}
23 changes: 22 additions & 1 deletion src/annotator/components/test/NotebookModal-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,13 @@ describe('NotebookModal', () => {

const outerSelector = '[data-testid="notebook-outer"]';

const createComponent = config => {
const createComponent = ({ attachTo, ...config } = {}) => {
const component = mount(
<NotebookModal
eventBus={eventBus}
config={{ notebookAppUrl: notebookURL, ...config }}
/>,
{ attachTo },
);
components.push(component);
return component;
Expand Down Expand Up @@ -106,6 +107,26 @@ describe('NotebookModal', () => {
assert.notEqual(iframe1.getDOMNode(), iframe3.getDOMNode());
});

it('focuses iframe when the notebook is opened', () => {
const container = document.createElement('div');
document.body.appendChild(container);

try {
const wrapper = createComponent({ attachTo: container });

// The body is initially focused
assert.equal(document.activeElement, document.body);

emitter.publish('openNotebook', '1');
wrapper.update();

// Once notebook is opened, focus transitions to iframe
assert.equal(document.activeElement, wrapper.find('iframe').getDOMNode());
} finally {
container.remove();
}
});

it('makes the document unscrollable on "openNotebook" event', () => {
createComponent();
act(() => {
Expand Down
10 changes: 5 additions & 5 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -1995,15 +1995,15 @@ __metadata:
languageName: node
linkType: hard

"@hypothesis/frontend-shared@npm:^7.2.0":
version: 7.2.0
resolution: "@hypothesis/frontend-shared@npm:7.2.0"
"@hypothesis/frontend-shared@npm:^7.3.0":
version: 7.3.0
resolution: "@hypothesis/frontend-shared@npm:7.3.0"
dependencies:
highlight.js: ^11.6.0
wouter-preact: ^2.10.0-alpha.1
peerDependencies:
preact: ^10.4.0
checksum: fe4b515e0f856dcb6c9876e52aba2a262d1bf6cebb13176b6ee2a676df719e2331106ee1595e2de057843fe7ebe8ac03f3a8f568fb03f50b3ea0622496da2ead
checksum: 5b295e0cb949c858d70c96ab24e7ec387d31e4a0e5bde6555ba70843cdceaae030a119e20841fa4924888db618c7fa47bdd4f83a3be65da889ad702506cc75f4
languageName: node
linkType: hard

Expand Down Expand Up @@ -7793,7 +7793,7 @@ __metadata:
"@babel/preset-react": ^7.0.0
"@babel/preset-typescript": ^7.16.7
"@hypothesis/frontend-build": ^3.0.0
"@hypothesis/frontend-shared": ^7.2.0
"@hypothesis/frontend-shared": ^7.3.0
"@hypothesis/frontend-testing": ^1.2.0
"@npmcli/arborist": ^7.0.0
"@octokit/rest": ^20.0.1
Expand Down

0 comments on commit e400d6c

Please sign in to comment.