Details elements aren't searchable - a11y issue

cassieevans commented 6 months ago

Have you read the Contributing Guidelines on issues?

[X] I have read the Contributing Guidelines on issues.

Prerequisites

[X] I'm using the latest version of Docusaurus.
[X] I have tried the npm run clear or yarn clear command.
[X] I have tried rm -rf node_modules yarn.lock package-lock.json and re-installing packages.
[X] I have tried creating a repro with https://new.docusaurus.io.
[X] I have read the console error message carefully (if applicable).

Description

Hey folks.

The way that the details elements are implemented in docusaurus currently is incompatible with searching the page with cmd + F.

This is quite a big deal for documentation.

Closed details elements are searchable in chrome https://chromestatus.com/feature/5032469667512320

I've tested in safari and firefox on my mac too and they also open on this page if I search for a word within https://developer.mozilla.org/en-US/docs/Web/HTML/Element/details

However, in my site and on the docusaurus site they aren't searchable due to the styling that animates the height change.

searching for "correlation" on this page yields no results https://docusaurus.io/docs/markdown-features

Reproducible demo

https://docusaurus.io/docs/markdown-features

Steps to reproduce

searching for "correlation" on this page yields no results

Expected behavior

the details element should open

Actual behavior

it doesn't

Your environment

Public source code:
Public site URL:
Docusaurus version used:
Environment name and version (e.g. Chrome 89, Node.js 16.4):
Operating system and version (e.g. Ubuntu 20.04.2 LTS):

Self-service

[ ] I'd be willing to fix this bug myself.

cassieevans commented 6 months ago

Another little note -

If I remove display none on the inner content div and then try and search the page - the browser finds the info and tries to toggle the details element open by adding an 'open' attribute.

Unfortunately the JS that runs this component seems to only change the "data-collapsed" attribute on click (maybe some other events too?)

A fix could be to listen to the onToggle event, then it would be aware that the browser was trying to open the details element.

slorber commented 6 months ago

Yes, I've also seen this problem. I thought it self-fixed somehow, because I saw it working recently, but apparently not 😅

Surprisingly this is a "progressive degradation" because if you turn JS off, it's not possible to search those details elements.

I'm looking into it right now but it looks like this component should probably not "control" the open prop.

cassieevans commented 6 months ago

Thanks Sébastien!

slorber commented 6 months ago

Here's a first attempt, it works but introduces other bugs and also not sure the experience is great because the keyword does not highlight until the animation completes 😅

https://github.com/facebook/docusaurus/pull/10057

This probably requires more work to figure this out 😅

Maybe until we find a solution we could expose an option to disable that "animated progressive enhancement" through swizzle? This way your details won't be animated but at least they would be accessible.

cassieevans commented 6 months ago

Yeah - I think that would be a good middle ground in the interim. Thanks for looking into it. 🙏

Man this means I'm gonna have to update to v3 doesn't it 🫠 Been putting that one off because of the React 18/animation cleanup implications.

slorber commented 6 months ago

Technically you can already swizzle the @theme/Details element, it's just a wrapper around our "internal lib component" so you could provide your own implementation for it even under v2.

Here's what our implementation is doing:

import React from 'react';
import clsx from 'clsx';
import {Details as DetailsGeneric} from '@docusaurus/theme-common/Details';
import type {Props} from '@theme/Details';

import styles from './styles.module.css';

const InfimaClasses = 'alert alert--info';

export default function Details({...props}: Props): JSX.Element {
  return (
    <DetailsGeneric
      {...props}
      className={clsx(InfimaClasses, styles.details, props.className)}
    />
  );
}

You could replace it with a simpler one:

import React from 'react';
import type {Props} from '@theme/Details';

const InfimaClasses = 'alert alert--info';

export default function Details({summary, ...props}: Props): JSX.Element {
  const summaryElement = React.isValidElement(summary) ? (
    summary
  ) : (
    <summary>{summary ?? 'Details'}</summary>
  );

  return (
    <details {...props} className={InfimaClasses}>
      {summaryElement}
      {props.children}
    </details>
  );
}

(an important detail here is that <summary> is received as props.summary, not as props.children[0] like you would expect)

The only difference would be that it will not be collapsible, and the arrow is not using a custom animated icon

If I provide something in v3, that would only be a "shortcut" to the workaround suggested above. I guess it's not really useful to provide considering it's quite easy to achieve on v2/v3 already. We should focus on solving the accessibility bug directly.

cassieevans commented 6 months ago

Ah, yeah that's a nice easy swizzle. I'll get on that. Thanks so much for the advice. Appreciated!

cassieevans commented 6 months ago

Got it all implemented with a little animated icon/animated expand and it's working with search - thanks so much!

slorber commented 1 month ago

Note for myself, in 2025 we might be able to progressively enhance and animate details element without and JS: https://nerdy.dev/open-and-close-transitions-for-the-details-element

I'm thinking of rebuilding a new Details component based on these new modern CSS attributes. We could expose it with an opt-in flag until browsers support those new CSS rules.

facebook / docusaurus