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(v2): fix contentTitle issues when markdown h1 title contains code blocks #4882

Merged
merged 11 commits into from
Jun 3, 2021
Merged
11 changes: 6 additions & 5 deletions packages/docusaurus-mdx-loader/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,7 @@ module.exports = async function docusaurusMdxLoader(fileString) {

const {frontMatter, content: contentWithTitle} = parseFrontMatter(fileString);

// By default, will remove the markdown title from the content
const {content} = parseMarkdownContentTitle(contentWithTitle, {
keepContentTitle: reqOptions.keepContentTitle,
});
const {content, contentTitle} = parseMarkdownContentTitle(contentWithTitle);

const hasFrontMatter = Object.keys(frontMatter).length > 0;

Expand Down Expand Up @@ -69,7 +66,11 @@ module.exports = async function docusaurusMdxLoader(fileString) {
return callback(err);
}

let exportStr = `export const frontMatter = ${stringifyObject(frontMatter)};`;
let exportStr = ``;
exportStr += `\nexport const frontMatter = ${stringifyObject(frontMatter)};`;
exportStr += `\nexport const contentTitle = ${stringifyObject(
contentTitle,
)};`;

// Read metadata for this MDX and export it.
if (options.metadataPath && typeof options.metadataPath === 'function') {
Expand Down
9 changes: 2 additions & 7 deletions packages/docusaurus-plugin-content-docs/src/docs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -203,12 +203,7 @@ export function processDocMetadata({
numberPrefixParser: options.numberPrefixParser,
});

// TODO expose both headingTitle+metaTitle to theme?
// Different fallbacks order on purpose!
// See https://github.com/facebook/docusaurus/issues/4665#issuecomment-825831367
const headingTitle: string = contentTitle ?? frontMatter.title ?? baseID;
// const metaTitle: string = frontMatter.title ?? contentTitle ?? baseID;

const title: string = frontMatter.title ?? contentTitle ?? baseID;
const description: string = frontMatter.description ?? excerpt ?? '';

const permalink = normalizeUrl([versionMetadata.versionPath, docSlug]);
Expand Down Expand Up @@ -246,7 +241,7 @@ export function processDocMetadata({
unversionedId,
id,
isDocsHomePage,
title: headingTitle,
title,
description,
source: aliasedSitePath(filePath, siteDir),
sourceDirName,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ declare module '@theme/DocItem' {
readonly frontMatter: FrontMatter;
readonly metadata: Metadata;
readonly toc: readonly TOCItem[];
readonly contentTitle: string | undefined;
(): JSX.Element;
};
};
Expand Down
1 change: 0 additions & 1 deletion packages/docusaurus-plugin-content-pages/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,6 @@ export default function pluginContentPages(
rehypePlugins,
beforeDefaultRehypePlugins,
beforeDefaultRemarkPlugins,
keepContentTitle: true,
staticDir: path.join(siteDir, STATIC_DIR_NAME),
// Note that metadataPath must be the same/in-sync as
// the path from createData for each MDX.
Expand Down
17 changes: 8 additions & 9 deletions packages/docusaurus-theme-classic/src/theme/DocItem/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import LastUpdated from '@theme/LastUpdated';
import type {Props} from '@theme/DocItem';
import TOC from '@theme/TOC';
import EditThisPage from '@theme/EditThisPage';
import {MainHeading} from '@theme/Heading';

import clsx from 'clsx';
import styles from './styles.module.css';
Expand Down Expand Up @@ -49,13 +50,15 @@ function DocItem(props: Props): JSX.Element {
// See https://github.com/facebook/docusaurus/issues/3362
const showVersionBadge = versions.length > 1;

// For meta title, using frontMatter.title in priority over a potential # title found in markdown
// See https://github.com/facebook/docusaurus/issues/4665#issuecomment-825831367
const metaTitle = frontMatter.title || title;
// We only add a title if:
// - user asks to hide it with frontmatter
// - the markdown content does not already contain a top-level h1 heading
const shouldAddTitle =
!hideTitle && typeof DocContent.contentTitle === 'undefined';

return (
<>
<Seo {...{title: metaTitle, description, keywords, image}} />
<Seo {...{title, description, keywords, image}} />

<div className="row">
<div
Expand All @@ -72,11 +75,7 @@ function DocItem(props: Props): JSX.Element {
</span>
</div>
)}
{!hideTitle && (
<header>
<h1 className={styles.docTitle}>{title}</h1>
</header>
)}
{shouldAddTitle && <MainHeading>{title}</MainHeading>}
<div className="markdown">
<DocContent />
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,6 @@
* LICENSE file in the root directory of this source tree.
*/

.docTitle {
font-size: 3rem;
margin-bottom: calc(var(--ifm-leading-desktop) * var(--ifm-leading));
}

.docItemContainer {
margin: 0 auto;
padding: 0 0.5rem;
Expand Down
20 changes: 19 additions & 1 deletion packages/docusaurus-theme-classic/src/theme/Heading/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,21 @@ import {useThemeConfig} from '@docusaurus/theme-common';
import './styles.css';
import styles from './styles.module.css';

const Heading = (Tag: HeadingType): ((props: Props) => JSX.Element) =>
type HeadingComponent = (props: Props) => JSX.Element;

export const MainHeading: HeadingComponent = function MainHeading({...props}) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For h1 found in MDX we don't add anchors.

This is a potential side-effect for a minority of users but using multiple h1 in a doc is not something they should do in the first place

return (
<header>
<h1 {...props} className={styles.h1Heading}>
{props.children}
</h1>
</header>
);
};

const createAnchorHeading = (
Tag: HeadingType,
): ((props: Props) => JSX.Element) =>
function TargetComponent({id, ...props}) {
const {
navbar: {hideOnScroll},
Expand Down Expand Up @@ -51,4 +65,8 @@ const Heading = (Tag: HeadingType): ((props: Props) => JSX.Element) =>
);
};

const Heading = (headingType: HeadingType): ((props: Props) => JSX.Element) => {
return headingType === 'h1' ? MainHeading : createAnchorHeading(headingType);
};

export default Heading;
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,8 @@
.enhancedAnchor {
top: calc(var(--ifm-navbar-height) * -1 - 0.5rem);
}

.h1Heading {
font-size: 3rem;
margin-bottom: calc(var(--ifm-leading-desktop) * var(--ifm-leading));
}
1 change: 1 addition & 0 deletions packages/docusaurus-theme-classic/src/types.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,7 @@ declare module '@theme/Heading' {

const Heading: (Tag: HeadingType) => (props: Props) => JSX.Element;
export default Heading;
export const MainHeading: (props: Props) => JSX.Element;
}

declare module '@theme/hooks/useAnnouncementBar' {
Expand Down
62 changes: 0 additions & 62 deletions packages/docusaurus-utils/src/__tests__/markdownParser.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -357,22 +357,6 @@ Lorem Ipsum
});
});

test('Should parse markdown h1 title at the top but keep it in content', () => {
const markdown = dedent`

# Markdown Title

Lorem Ipsum

`;
expect(
parseMarkdownContentTitle(markdown, {keepContentTitle: true}),
).toEqual({
content: markdown.trim(),
contentTitle: 'Markdown Title',
});
});

test('Should not parse markdown h1 title in the middle of a doc', () => {
const markdown = dedent`

Expand Down Expand Up @@ -544,34 +528,6 @@ describe('parseMarkdownString', () => {
`);
});

test('should not warn for duplicate title if keepContentTitle=true', () => {
expect(
parseMarkdownString(
dedent`
---
title: Frontmatter title
---

# Markdown Title

Some text
`,
{keepContentTitle: true},
),
).toMatchInlineSnapshot(`
Object {
"content": "# Markdown Title

Some text",
"contentTitle": "Markdown Title",
"excerpt": "Some text",
"frontMatter": Object {
"title": "Frontmatter title",
},
}
`);
});

test('should not warn for duplicate title if markdown title is not at the top', () => {
expect(
parseMarkdownString(dedent`
Expand All @@ -597,24 +553,6 @@ describe('parseMarkdownString', () => {
`);
});

test('should parse markdown title and keep it in content', () => {
expect(
parseMarkdownString(
dedent`
# Markdown Title
`,
{keepContentTitle: true},
),
).toMatchInlineSnapshot(`
Object {
"content": "# Markdown Title",
"contentTitle": "Markdown Title",
"excerpt": undefined,
"frontMatter": Object {},
}
`);
});

test('should delete only first heading', () => {
expect(
parseMarkdownString(dedent`
Expand Down
33 changes: 13 additions & 20 deletions packages/docusaurus-utils/src/markdownParser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,12 +78,17 @@ export function parseFrontMatter(
};
}

// Unwrap possible inline code blocks (# `config.js`)
function cleanContentTitle(contentTitle: string): string {
if (contentTitle.startsWith('`') && contentTitle.endsWith('`')) {
return contentTitle.substring(1, contentTitle.length - 1);
}
return contentTitle;
}

export function parseMarkdownContentTitle(
contentUntrimmed: string,
options?: {keepContentTitle?: boolean},
): {content: string; contentTitle: string | undefined} {
const keepContentTitleOption = options?.keepContentTitle ?? false;

const content = contentUntrimmed.trim();

const IMPORT_STATEMENT = /import\s+(([\w*{}\s\n,]+)from\s+)?["'\s]([@\w/_.-]+)["'\s];?|\n/
Expand All @@ -106,16 +111,12 @@ export function parseMarkdownContentTitle(

if (!pattern || !title) {
return {content, contentTitle: undefined};
} else {
return {
content,
contentTitle: cleanContentTitle(title.trim()).trim(),
};
}

const newContent = keepContentTitleOption
? content
: content.replace(pattern, '');

return {
content: newContent.trim(),
contentTitle: title.trim(),
};
}

type ParsedMarkdown = {
Expand All @@ -127,22 +128,14 @@ type ParsedMarkdown = {

export function parseMarkdownString(
markdownFileContent: string,
options?: {
keepContentTitle?: boolean;
},
): ParsedMarkdown {
try {
const keepContentTitle = options?.keepContentTitle ?? false;

const {frontMatter, content: contentWithoutFrontMatter} = parseFrontMatter(
markdownFileContent,
);

const {content, contentTitle} = parseMarkdownContentTitle(
contentWithoutFrontMatter,
{
keepContentTitle,
},
);

const excerpt = createExcerpt(content);
Expand Down
3 changes: 2 additions & 1 deletion website/docs/api/docusaurus.config.js.md
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
---
id: docusaurus.config.js
title: docusaurus.config.js
description: API reference for Docusaurus configuration file.
slug: /docusaurus.config.js
---

# `docusaurus.config.js`
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dogfood on our site


## Overview {#overview}

`docusaurus.config.js` contains configurations for your site and is placed in the root directory of your site.
Expand Down