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

Details elements aren't searchable - a11y issue #10055

Open
6 of 7 tasks
cassieevans opened this issue Apr 18, 2024 · 8 comments
Open
6 of 7 tasks

Details elements aren't searchable - a11y issue #10055

cassieevans opened this issue Apr 18, 2024 · 8 comments
Labels
bug An error in the Docusaurus core causing instability or issues with its execution

Comments

@cassieevans
Copy link

cassieevans commented Apr 18, 2024

Have you read the Contributing Guidelines on issues?

Prerequisites

  • I'm using the latest version of Docusaurus.
  • I have tried the npm run clear or yarn clear command.
  • I have tried rm -rf node_modules yarn.lock package-lock.json and re-installing packages.
  • I have tried creating a repro with https://new.docusaurus.io.
  • 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 cassieevans added bug An error in the Docusaurus core causing instability or issues with its execution status: needs triage This issue has not been triaged by maintainers labels Apr 18, 2024
@slorber slorber removed the status: needs triage This issue has not been triaged by maintainers label Apr 18, 2024
@cassieevans
Copy link
Author

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
Copy link
Collaborator

slorber commented Apr 18, 2024

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
Copy link
Author

Thanks Sébastien!

@slorber
Copy link
Collaborator

slorber commented Apr 18, 2024

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 😅

#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
Copy link
Author

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
Copy link
Collaborator

slorber commented Apr 18, 2024

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

CleanShot 2024-04-18 at 16 54 37


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
Copy link
Author

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

@cassieevans
Copy link
Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug An error in the Docusaurus core causing instability or issues with its execution
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants