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): forward className prop in theme-classic's Heading #8350

Merged

Conversation

JoshuaKGoldberg
Copy link
Contributor

@JoshuaKGoldberg JoshuaKGoldberg commented Nov 16, 2022

Pre-flight checklist

  • I have read the Contributing Guidelines on pull requests.
  • If this is a code change: I have written unit tests and/or added dogfooding pages to fully verify the new behavior.
  • [ ] If this is a new API or substantial change: the PR has an accompanying issue (closes #0000) and the maintainers have approved on my working plan.

Motivation

See #8327.

Apologies for taking a good first issue after I've already sent a couple PRs to this repo. I just was excited about this one 😄

Test Plan

I tried this out locally on the typescript-eslint/typescript-eslint#5951 PR and it worked.

Test links

I don't see any existing tests for this. What would you like me to do?

Related issues/PRs

Fixes #8327.

@facebook-github-bot facebook-github-bot added the CLA Signed Signed Facebook CLA label Nov 16, 2022
@netlify
Copy link

netlify bot commented Nov 16, 2022

[V2]

Built without sensitive environment variables

Name Link
🔨 Latest commit f2aedca
🔍 Latest deploy log https://app.netlify.com/sites/docusaurus-2/deploys/63750e8129908200092cfbf9
😎 Deploy Preview https://deploy-preview-8350--docusaurus-2.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@github-actions
Copy link

⚡️ Lighthouse report for the deploy preview of this PR

URL Performance Accessibility Best Practices SEO PWA Report
/ 🟠 85 🟢 97 🟢 100 🟢 100 🟢 90 Report
/docs/installation 🟠 75 🟢 100 🟢 100 🟢 100 🟢 90 Report

@slorber slorber added the pr: bug fix This PR fixes a bug in a past release. label Nov 16, 2022
@slorber slorber marked this pull request as ready for review November 16, 2022 16:29
@slorber slorber self-requested a review as a code owner November 16, 2022 16:29
@slorber
Copy link
Collaborator

slorber commented Nov 16, 2022

:D

Apparently this is the only thing that should be done.

The TS side already existed and we just did not merge classes properly

  export interface Props extends ComponentProps<HeadingType> {
    readonly as: HeadingType;
  }

We don't have good tests on the UI, will add a storybook later but it's ok for now 👍

@slorber slorber changed the title fix: forward className prop in theme-classic's Heading fix(theme): forward className prop in theme-classic's Heading Nov 16, 2022
@slorber slorber merged commit e756d73 into facebook:main Nov 16, 2022
@JoshuaKGoldberg JoshuaKGoldberg deleted the theme-classic-heading-className-prop branch November 16, 2022 16:55
@slorber slorber added the to backport This PR is planned to be backported to a stable version of Docusaurus label Nov 17, 2022
@slorber slorber added backported This PR has been backported to a stable version of Docusaurus and removed to backport This PR is planned to be backported to a stable version of Docusaurus labels Jan 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backported This PR has been backported to a stable version of Docusaurus CLA Signed Signed Facebook CLA pr: bug fix This PR fixes a bug in a past release.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

docusaurus-theme-classic > Heading's className prop does nothing
3 participants