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

Make sure sidebar panels are focused when open, and focus is restored when closed #5390

Merged
merged 2 commits into from
Apr 17, 2023
Merged
Show file tree
Hide file tree
Changes from all 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
26 changes: 16 additions & 10 deletions src/sidebar/components/SidebarPanel.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Panel } from '@hypothesis/frontend-shared/lib/next';
import { Dialog } from '@hypothesis/frontend-shared/lib/next';
import type { IconComponent } from '@hypothesis/frontend-shared/lib/types';
import type { ComponentChildren } from 'preact';
import { useCallback, useEffect, useRef } from 'preact/hooks';
Expand Down Expand Up @@ -47,9 +47,7 @@ export default function SidebarPanel({
if (panelIsActive && panelElement.current) {
scrollIntoView(panelElement.current);
}
if (typeof onActiveChanged === 'function') {
onActiveChanged(panelIsActive);
}
onActiveChanged?.(panelIsActive);
}
}, [panelIsActive, onActiveChanged]);

Expand All @@ -58,12 +56,20 @@ export default function SidebarPanel({
}, [store, panelName]);

return (
<Slider visible={panelIsActive}>
<div ref={panelElement} className="mb-4">
<Panel title={title} icon={icon} onClose={closePanel}>
<>
{panelIsActive && (
<Dialog
restoreFocus
ref={panelElement}
classes="mb-4"
title={title}
icon={icon}
onClose={closePanel}
transitionComponent={Slider}
>
{children}
</Panel>
</div>
</Slider>
</Dialog>
)}
</>
);
}
15 changes: 12 additions & 3 deletions src/sidebar/components/Slider.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@ export type SliderProps = {

/** Whether the content should be visible or not. */
visible: boolean;

/** Invoked once the open/close transitions have finished */
onTransitionEnd?: (direction: 'in' | 'out') => void;
};

/**
Expand All @@ -17,9 +20,13 @@ export type SliderProps = {
* DOM using `display: none` so it does not appear in the keyboard navigation
* order.
*
* Currently the only reveal/expand direction supported is top-down.
* Currently, the only reveal/expand direction supported is top-down.
*/
export default function Slider({ children, visible }: SliderProps) {
export default function Slider({
children,
visible,
onTransitionEnd,
}: SliderProps) {
const containerRef = useRef<HTMLDivElement | null>(null);
const [containerHeight, setContainerHeight] = useState(visible ? 'auto' : 0);

Expand Down Expand Up @@ -68,13 +75,15 @@ export default function Slider({ children, visible }: SliderProps) {
const handleTransitionEnd = useCallback(() => {
if (visible) {
setContainerHeight('auto');
onTransitionEnd?.('in');
} else {
// When the collapse animation completes, stop rendering the content so
// that the browser has fewer nodes to render and the content is removed
// from keyboard navigation.
setContentVisible(false);
onTransitionEnd?.('out');
}
}, [setContainerHeight, visible]);
}, [setContainerHeight, visible, onTransitionEnd]);

const isFullyVisible = containerHeight === 'auto';

Expand Down
12 changes: 6 additions & 6 deletions src/sidebar/components/test/SidebarPanel-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ describe('SidebarPanel', () => {
fakeScrollIntoView = sinon.stub();

fakeStore = {
isSidebarPanelOpen: sinon.stub().returns(false),
isSidebarPanelOpen: sinon.stub().returns(true),
toggleSidebarPanel: sinon.stub(),
};

Expand All @@ -36,16 +36,16 @@ describe('SidebarPanel', () => {
icon: 'restricted',
});

const panel = wrapper.find('Panel');
const dialog = wrapper.find('Dialog');

assert.equal(panel.props().icon, 'restricted');
assert.equal(panel.props().title, 'My Panel');
assert.equal(dialog.props().icon, 'restricted');
assert.equal(dialog.props().title, 'My Panel');
});

it('provides an `onClose` handler that closes the panel', () => {
const wrapper = createSidebarPanel({ panelName: 'flibberty' });

wrapper.find('Panel').props().onClose();
wrapper.find('Dialog').props().onClose();

assert.calledWith(fakeStore.toggleSidebarPanel, 'flibberty', false);
});
Expand All @@ -59,7 +59,7 @@ describe('SidebarPanel', () => {
it('hides content if not active', () => {
fakeStore.isSidebarPanelOpen.returns(false);
const wrapper = createSidebarPanel();
assert.isFalse(wrapper.find('Slider').prop('visible'));
assert.isFalse(wrapper.find('Dialog').exists());
});

context('when panel state changes', () => {
Expand Down