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

docusaurus-theme-classic > Heading's className prop does nothing #8327

Closed
7 tasks done
JoshuaKGoldberg opened this issue Nov 12, 2022 · 1 comment · Fixed by #8350
Closed
7 tasks done

docusaurus-theme-classic > Heading's className prop does nothing #8327

JoshuaKGoldberg opened this issue Nov 12, 2022 · 1 comment · Fixed by #8350
Labels
bug An error in the Docusaurus core causing instability or issues with its execution good first issue If you are just getting started with Docusaurus, this issue should be a good place to begin.

Comments

@JoshuaKGoldberg
Copy link
Contributor

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

The <Heading> component in docusaurus-theme-classic can be given a className prop:

<Heading as="h2" className="something">Hi!</Heading>

However, the component's manual className=clsx(...) prop ignores props.className:

<As
{...props}
className={clsx(
'anchor',
hideOnScroll
? styles.anchorWithHideOnScrollNavbar
: styles.anchorWithStickyNavbar,
)}

This means any className added in user-land is ignored.

Reproducible demo

https://github.com/typescript-eslint/typescript-eslint/pull/5951/files#diff-3b034ddb6525c7b93f97ced8a696c78c1837f072b9d1e70c46049675e0e09bf4R112-R116

Steps to reproduce

  1. Checkout that PR -> yarn -> yarn start
  2. Change
    ...
    to ...
  3. Observe that the site page does not center the heading text

Expected behavior

The className should be added to the clsx in Heading.

Actual behavior

The className is ignored.

Your environment

Self-service

  • I'd be willing to fix this bug myself.
@JoshuaKGoldberg JoshuaKGoldberg 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 Nov 12, 2022
@slorber slorber added good first issue If you are just getting started with Docusaurus, this issue should be a good place to begin. and removed status: needs triage This issue has not been triaged by maintainers labels Nov 16, 2022
@slorber
Copy link
Collaborator

slorber commented Nov 16, 2022

👍 ok to forward the Heading className prop

Good first issue for anyone willing to contribute - please submit a PR directly, and do not claim the issue


Note historically most components did not forward className in our theme, and it's an ongoing effort to add className forwarding everywhere it makes sense (and maybe later forwarding refs? although it's a more niche use-case).

For example, it can be useful to add a custom className by simply wrapping instead of ejecting, also enabling you to use CSS modules

import styles from "./styles"

function HeadingWrapper(props) {
  return <Heading {...props} className={clsx(styles.myCustomClassName, props.className)}/>
}

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 good first issue If you are just getting started with Docusaurus, this issue should be a good place to begin.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants