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

Don't replace static assets linking in fenced code blocks #864

Merged
merged 6 commits into from
Jul 21, 2018

Conversation

endiliey
Copy link
Contributor

Motivation

Fix #861 and #857 (comment)

In our code, we replace all occurences of ](assets/ to ]({baseUrl}/docs/assets/
https://github.com/facebook/Docusaurus/blob/22f3a85a494ddf46bc29964a7989335a7dba4a9a/lib/server/docs.js#L79-L83

However, this cause a side effect to our documentation.

What we really mean

We want to tell people that they can link their assets by doing something like

![alt-text](assets/doc-image.png)

Reference:
https://github.com/facebook/Docusaurus/blob/master/docs/api-doc-markdown.md
correct-documentation

Misleading Documentation
However, the ![alt-text](assets/doc-image.png) is replaced into ![alt-text](/docs/assets/doc-image.png)

https://docusaurus.io/docs/en/next/doc-markdown#linking-to-images-and-other-assets
wrong-documentation

Solution: We should only replace static assets linking if it is not inside a fenced block

Have you read the Contributing Guidelines on pull requests?

Yes

Test Plan

  1. Correct documentation

after

  1. All added new tests passes

@facebook-github-bot facebook-github-bot added the CLA Signed Signed Facebook CLA label Jul 20, 2018
@endiliey endiliey requested review from yangshun and JoelMarcey July 20, 2018 13:08
@docusaurus-bot
Copy link
Contributor

docusaurus-bot commented Jul 20, 2018

Deploy preview for docusaurus-preview ready!

Built with commit a040545

https://deploy-preview-864--docusaurus-preview.netlify.com

Copy link
Contributor

@JoelMarcey JoelMarcey left a comment

Choose a reason for hiding this comment

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

💯

@endiliey endiliey added the status: don't merge yet This PR is completed, but we are still waiting for other things to be ready. label Jul 21, 2018
@endiliey endiliey removed the status: don't merge yet This PR is completed, but we are still waiting for other things to be ready. label Jul 21, 2018
@endiliey endiliey merged commit d42ecb9 into facebook:master Jul 21, 2018
@endiliey endiliey deleted the replaceassets branch July 21, 2018 13:54
@JoelMarcey JoelMarcey mentioned this pull request Aug 6, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed Signed Facebook CLA
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants