Skip to content

Commit

Permalink
Display notebook in a native dialog when possible
Browse files Browse the repository at this point in the history
  • Loading branch information
acelaya committed Feb 8, 2024
1 parent c19a5a2 commit 7d05f12
Show file tree
Hide file tree
Showing 2 changed files with 169 additions and 40 deletions.
133 changes: 105 additions & 28 deletions src/annotator/components/NotebookModal.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { IconButton, CancelIcon } from '@hypothesis/frontend-shared';
import classnames from 'classnames';
import { useEffect, useRef, useState } from 'preact/hooks';
import type { ComponentChildren } from 'preact';
import { useEffect, useMemo, useRef, useState } from 'preact/hooks';

import { addConfigFragment } from '../../shared/config-fragment';
import { createAppConfig } from '../config/app';
Expand All @@ -27,7 +28,7 @@ function NotebookIframe({ config, groupId }: NotebookIframeProps) {
const notebookAppSrc = addConfigFragment(config.notebookAppUrl, {
...createAppConfig(config.notebookAppUrl, config),

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

Expand All @@ -40,9 +41,86 @@ function NotebookIframe({ config, groupId }: NotebookIframeProps) {
/>
);
}

/**
* Checks if the browser supports native modal dialogs
*/
function isModalDialogSupported(document: Document) {
const dialog = document.createElement('dialog');
return typeof dialog.showModal === 'function';
}

export type NotebookModalProps = {
eventBus: EventBus;
config: NotebookConfig;

/** Test seam */
document_?: Document;
};

type WrapperProps = { isHidden: boolean; children: ComponentChildren };

const DialogWrapper = ({ isHidden, children }: WrapperProps) => {
const dialogRef = useRef<HTMLDialogElement | null>(null);

useEffect(() => {
if (isHidden) {
dialogRef.current?.close();
} else {
dialogRef.current?.showModal();
}
}, [isHidden]);

// Prevent the dialog to be closed when `Esc` is pressed, to keep previous
// behavior
useEffect(() => {
const dialogElement = dialogRef.current;

dialogElement?.addEventListener('cancel', event => {
event.preventDefault();

Check warning on line 80 in src/annotator/components/NotebookModal.tsx

View check run for this annotation

Codecov / codecov/patch

src/annotator/components/NotebookModal.tsx#L80

Added line #L80 was not covered by tests
});

return () => {
dialogElement?.removeEventListener('cancel', event => {
event.preventDefault();

Check warning on line 85 in src/annotator/components/NotebookModal.tsx

View check run for this annotation

Codecov / codecov/patch

src/annotator/components/NotebookModal.tsx#L85

Added line #L85 was not covered by tests
});
};
}, []);

return (
<dialog
ref={dialogRef}
className={classnames(
// Dialogs seem to have a default max-width and max-height of
// calc(100% - 38px) that we want to overwrite
'w-full h-full max-w-[calc(100%-1.5rem)] max-h-[calc(100%-1.5rem)]',
'relative backdrop:bg-black/50',
)}
data-testid="notebook-outer"
>
{children}
</dialog>
);
};

/**
* Temporary fallback used in browsers not supporting `dialog` element.
* It can be removed once all browsers we support can use it.
*/
const LegacyWrapper = ({ isHidden, children }: WrapperProps) => {
return (
<div
className={classnames(
'fixed z-max top-0 left-0 right-0 bottom-0 p-3 bg-black/50',
{ hidden: isHidden },
)}
data-testid="notebook-outer"
>
<div className="relative w-full h-full" data-testid="notebook-inner">
{children}
</div>
</div>
);
};

/**
Expand All @@ -51,6 +129,8 @@ export type NotebookModalProps = {
export default function NotebookModal({
eventBus,
config,
/* istanbul ignore next - test seam */
document_ = document,
}: NotebookModalProps) {
// Temporary solution: while there is no mechanism to sync new annotations in
// the notebook, we force re-rendering of the iframe on every 'openNotebook'
Expand All @@ -62,6 +142,11 @@ export default function NotebookModal({
const originalDocumentOverflowStyle = useRef('');
const emitterRef = useRef<Emitter | null>(null);

const Wrapper = useMemo(
() => (isModalDialogSupported(document_) ? DialogWrapper : LegacyWrapper),
[document_],
);

// Stores the original overflow CSS property of document.body and reset it
// when the component is destroyed
useEffect(() => {
Expand Down Expand Up @@ -106,32 +191,24 @@ 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 },
)}
data-testid="notebook-outer"
>
<div className="relative w-full h-full" data-testid="notebook-inner">
<div className="absolute right-0 m-3">
<IconButton
title="Close the Notebook"
onClick={onClose}
variant="dark"
classes={classnames(
// Remove the dark variant's background color to avoid
// interfering with modal overlays. Re-activate the dark variant's
// background color on hover.
// See https://github.com/hypothesis/client/issues/3676
'!bg-transparent enabled:hover:!bg-grey-3',
)}
>
<CancelIcon className="w-4 h-4" />
</IconButton>
</div>
<NotebookIframe key={iframeKey} config={config} groupId={groupId} />
<Wrapper isHidden={isHidden}>
<div className="absolute right-0 m-3">
<IconButton
title="Close the Notebook"
onClick={onClose}
variant="dark"
classes={classnames(
// Remove the dark variant's background color to avoid
// interfering with modal overlays. Re-activate the dark variant's
// background color on hover.
// See https://github.com/hypothesis/client/issues/3676
'!bg-transparent enabled:hover:!bg-grey-3',
)}
>
<CancelIcon className="w-4 h-4" />
</IconButton>
</div>
</div>
<NotebookIframe key={iframeKey} config={config} groupId={groupId} />
</Wrapper>
);
}
76 changes: 64 additions & 12 deletions src/annotator/components/test/NotebookModal-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,24 +9,32 @@ describe('NotebookModal', () => {
const notebookURL = 'https://test.hypothes.is/notebook';

let components;
let containers;
let eventBus;
let emitter;

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

const createComponent = config => {
const createComponent = (config, fakeDocument) => {
const attachTo = document.createElement('div');
document.body.appendChild(attachTo);
containers.push(attachTo);

const component = mount(
<NotebookModal
eventBus={eventBus}
config={{ notebookAppUrl: notebookURL, ...config }}
document_={fakeDocument}
/>,
{ attachTo },
);
components.push(component);
return component;
};

beforeEach(() => {
components = [];
containers = [];
eventBus = new EventBus();
emitter = eventBus.createEmitter();

Expand All @@ -43,6 +51,7 @@ describe('NotebookModal', () => {

afterEach(() => {
components.forEach(component => component.unmount());
containers.forEach(container => container.remove());
$imports.$restore();
});

Expand Down Expand Up @@ -114,23 +123,66 @@ describe('NotebookModal', () => {
assert.equal(document.body.style.overflow, 'hidden');
});

it('hides modal on closing', () => {
const wrapper = createComponent();
context('when modal dialog is not supported', () => {
let fakeDocument;

emitter.publish('openNotebook', 'myGroup');
wrapper.update();
beforeEach(() => {
fakeDocument = {
createElement: sinon.stub().returns({}),
};
});

let outer = wrapper.find(outerSelector);
assert.isFalse(outer.hasClass('hidden'));
it('does not render a dialog element', () => {
const wrapper = createComponent({}, fakeDocument);

act(() => {
wrapper.find('IconButton').prop('onClick')();
emitter.publish('openNotebook', 'myGroup');
wrapper.update();

assert.isFalse(wrapper.exists('dialog'));
});
wrapper.update();

outer = wrapper.find(outerSelector);
it('hides modal on closing', () => {
const wrapper = createComponent({}, fakeDocument);

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

assert.isTrue(outer.hasClass('hidden'));
let outer = wrapper.find(outerSelector);
assert.isFalse(outer.hasClass('hidden'));

act(() => {
wrapper.find('IconButton').prop('onClick')();
});
wrapper.update();

outer = wrapper.find(outerSelector);

assert.isTrue(outer.hasClass('hidden'));
});
});

context('when modal dialog is supported', () => {
it('renders a dialog element', () => {
const wrapper = createComponent({});

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

assert.isTrue(wrapper.exists('dialog'));
});

it('opens and closes native dialog', () => {
const wrapper = createComponent({});
const isDialogOpen = () => wrapper.find('dialog').getDOMNode().open;

act(() => emitter.publish('openNotebook', 'myGroup'));
wrapper.update();
assert.isTrue(isDialogOpen());

act(() => wrapper.find('IconButton').prop('onClick')());
wrapper.update();
assert.isFalse(isDialogOpen());
});
});

it('resets document scrollability on closing the modal', () => {
Expand Down

0 comments on commit 7d05f12

Please sign in to comment.