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(theme): fix details element accessibility/search #10057

Closed
wants to merge 1 commit into from
Closed
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
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ export function useCollapsible({
}

const CollapsedStyles = {
display: 'none',
display: 'block',
overflow: 'hidden',
height: '0px',
} as const;
Expand Down
37 changes: 23 additions & 14 deletions packages/docusaurus-theme-common/src/components/Details/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,7 @@
* LICENSE file in the root directory of this source tree.
*/

import React, {
useRef,
useState,
type ComponentProps,
type ReactElement,
} from 'react';
import React, {useRef, type ComponentProps, type ReactElement} from 'react';
import clsx from 'clsx';
import useBrokenLinks from '@docusaurus/useBrokenLinks';
import useIsBrowser from '@docusaurus/useIsBrowser';
Expand Down Expand Up @@ -56,22 +51,28 @@ export function Details({
const {collapsed, setCollapsed} = useCollapsible({
initialState: !props.open,
});
// Use a separate state for the actual details prop, because it must be set
// only after animation completes, otherwise close animations won't work
const [open, setOpen] = useState(props.open);

const summaryElement = React.isValidElement(summary) ? (
summary
) : (
<summary>{summary ?? 'Details'}</summary>
);

function toggle() {
if (collapsed) {
setCollapsed(false);
detailsRef.current!.open = true;
} else {
setCollapsed(true);
// Don't remove "open" it breaks close animation!
}
}

return (
// eslint-disable-next-line jsx-a11y/click-events-have-key-events, jsx-a11y/no-noninteractive-element-interactions
<details
{...props}
ref={detailsRef}
open={open}
data-collapsed={collapsed}
className={clsx(
styles.details,
Expand All @@ -85,7 +86,16 @@ export function Details({
e.preventDefault();
}
}}
onToggle={(e) => {
console.log('toggle', e.currentTarget.open);

// For isolation of multiple nested details/summary
e.stopPropagation();
e.preventDefault();
toggle();
}}
onClick={(e) => {
console.log('click', e);
e.stopPropagation(); // For isolation of multiple nested details/summary
const target = e.target as HTMLElement;
const shouldToggle =
Expand All @@ -96,11 +106,10 @@ export function Details({
e.preventDefault();
if (collapsed) {
setCollapsed(false);
setOpen(true);
detailsRef.current!.open = true;
} else {
setCollapsed(true);
// Don't do this, it breaks close animation!
// setOpen(false);
// Don't remove "open" it breaks close animation!
}
}}>
{summaryElement}
Expand All @@ -111,7 +120,7 @@ export function Details({
disableSSRStyle // Allows component to work fine even with JS disabled!
onCollapseTransitionEnd={(newCollapsed) => {
setCollapsed(newCollapsed);
setOpen(!newCollapsed);
detailsRef.current!.open = !newCollapsed;
}}>
<div className={styles.collapsibleContent}>{children}</div>
</Collapsible>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ CSS variables, meant to be overridden by final theme
/* When JS disabled/failed to load: we use the open property for arrow animation: */
.details[open]:not(.isBrowser) > summary::before,
/* When JS works: we use the data-attribute for arrow animation */
.details[data-collapsed='false'].isBrowser > summary::before {
.details[open].isBrowser > summary::before {
transform: rotate(90deg);
}

Expand Down
Loading