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.

Sign up for GitHub

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

Jump to bottom

fix(v1): markdown content and toc should render the same #2022

Merged
merged 3 commits into from Nov 21, 2019
Merged

fix(v1): markdown content and toc should render the same #2022

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d2dc9a7
fix(v1): autogenerated_toc and sidebar toc should render the same
endiliey Nov 21, 2019
7988079
revert test docs
endiliey Nov 21, 2019
8ebd111
yarn.lock
endiliey Nov 21, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
45 changes: 29 additions & 16 deletions packages/docusaurus-1.x/lib/core/__tests__/toc.test.js
View file
Open in desktop
Expand Up @@ -39,22 +39,35 @@ describe('getTOC', () => {
expect(headingsJson).toContain('4th level headings');
});

describe('stripping of HTML', () => {
test('correctly removes', () => {
const headings = getTOC(`## <a name="foo"></a> Foo`, 'h2', []);

expect(headings[0].hashLink).toEqual('a-namefooa-foo');
expect(headings[0].rawContent).toEqual(`<a name="foo"></a> Foo`);
expect(headings[0].content).toEqual('Foo');
});

test('retains formatting from Markdown', () => {
const headings = getTOC(`## <a name="foo"></a> _Foo_`, 'h2', []);

expect(headings[0].hashLink).toEqual('a-namefooa-_foo_');
expect(headings[0].rawContent).toEqual(`<a name="foo"></a> _Foo_`);
expect(headings[0].content).toEqual('<em>Foo</em>');
});
test('html tag in source', () => {
const headings = getTOC(`## <a name="foo"></a> Foo`, 'h2', []);

expect(headings[0].hashLink).toEqual('a-namefooa-foo');
expect(headings[0].rawContent).toEqual(`<a name="foo"></a> Foo`);
expect(headings[0].content).toEqual(`<a name="foo"></a> Foo`);
});

test('transform markdown syntax to html syntax', () => {
const headings = getTOC(`## <a name="foo"></a> _Foo_`, 'h2', []);

expect(headings[0].hashLink).toEqual('a-namefooa-_foo_');
expect(headings[0].rawContent).toEqual(`<a name="foo"></a> _Foo_`);
expect(headings[0].content).toEqual(`<a name="foo"></a> <em>Foo</em>`);

const headings2 = getTOC(`## **Foo**`, 'h2', []);

expect(headings2[0].hashLink).toEqual('foo');
expect(headings2[0].rawContent).toEqual(`**Foo**`);
expect(headings2[0].content).toEqual(`<strong>Foo</strong>`);
});

test('does not strip tags randomly', () => {
// eslint-disable-next-line no-useless-escape
const headings = getTOC(`## function1 [array\<string>]`, 'h2', []);

expect(headings[0].hashLink).toEqual('function1-arraystring');
expect(headings[0].rawContent).toEqual(`function1 [array<string>]`);
expect(headings[0].content).toEqual(`function1 [array<string>]`);
});
});

Expand Down
9 changes: 5 additions & 4 deletions packages/docusaurus-1.x/lib/core/toc.js
View file
Open in desktop
Expand Up @@ -7,7 +7,6 @@

const {Remarkable} = require('remarkable');
const mdToc = require('markdown-toc');
const striptags = require('striptags');
const GithubSlugger = require('github-slugger');
const toSlug = require('./toSlug');

Expand All @@ -27,16 +26,18 @@ function getTOC(content, headingTags = 'h2', subHeadingTags = 'h3') {
? [].concat(subHeadingTags).map(tagToLevel)
: [];
const allowedHeadingLevels = headingLevels.concat(subHeadingLevels);
const md = new Remarkable();
const md = new Remarkable({
// Enable HTML tags in source (same as './renderMarkdown.js')
html: true,
});
const headings = mdToc(content).json;
const toc = [];
const slugger = new GithubSlugger();
let current;

headings.forEach(heading => {
const rawContent = heading.content;
const safeContent = striptags(rawContent);
Copy link
Contributor

Choose a reason for hiding this comment

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

We should find out when/why this was added. We could be regressing on a bug.

Copy link
Contributor Author

@endiliey endiliey Nov 21, 2019 •
edited

Choose a reason for hiding this comment

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

It's because of #1762 which was a wrong solution.

So the reason why #2021 happens (inconsistent) is because in right TOC, we try to strip tags, and then render with remarkable.

In markdown itself, we only render with remarkable.

The remarkable render function is also different.

docusaurus/packages/docusaurus-1.x/lib/core/renderMarkdown.js

Line 86 in 91902ed

html: true, 

new Remarkable({html: true});

vs

docusaurus/packages/docusaurus-1.x/lib/core/toc.js

Line 30 in 91902ed

const md = new Remarkable();

which is equivalent to

new Remarkable({
  html:         false,  // this is default

Pretty sure this is 99.99% the right fix

Copy link
Contributor Author

@endiliey endiliey Nov 21, 2019 •
edited

Choose a reason for hiding this comment

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

#1762 attempted to fix a bug whereby

## Referencing Site Documents <span id="jump">

image

Should be rendered on the right without the span text in TOC.

But its causing inconsistent content.

This PR on other hand fixed that bug but is still consistent

const rendered = md.renderInline(safeContent);
const rendered = md.renderInline(rawContent);

const hashLink = toSlug(rawContent, slugger);
if (!allowedHeadingLevels.includes(heading.lvl)) {
Expand Down
1 change: 0 additions & 1 deletion packages/docusaurus-1.x/package.json
View file
Open in desktop
Expand Up @@ -70,7 +70,6 @@
"request": "^2.88.0",
"shelljs": "^0.8.3",
"sitemap": "^3.2.2",
"striptags": "^3.1.1",
"tcp-port-used": "^1.0.1",
"tiny-lr": "^1.1.1",
"tree-node-cli": "^1.2.5",
Expand Down