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): tidy up Markdown page layout #4917

Merged
merged 1 commit into from
Jun 10, 2021
Merged

Conversation

lex111
Copy link
Contributor

@lex111 lex111 commented Jun 6, 2021

Motivation

Quite strange looking now markdown page, I suppose it should be aligned to center like on all the other pages (see screenshots below).

Have you read the Contributing Guidelines on pull requests?

Yes

Test Plan

Before
(https://docusaurus.io/examples/markdownPageExample)
After
(https://deploy-preview-4917--docusaurus-2.netlify.app/examples/markdownPageExample)
image image

Related PRs

(If this PR adds or changes functionality, please take some time to update the docs at https://github.com/facebook/docusaurus, and link to your PR here.)

@lex111 lex111 added the pr: bug fix This PR fixes a bug in a past release. label Jun 6, 2021
@lex111 lex111 requested a review from slorber as a code owner June 6, 2021 12:36
@facebook-github-bot facebook-github-bot added the CLA Signed Signed Facebook CLA label Jun 6, 2021
@netlify
Copy link

netlify bot commented Jun 6, 2021

✔️ [V2]

🔨 Explore the source changes: 611e063

🔍 Inspect the deploy log: https://app.netlify.com/sites/docusaurus-2/deploys/60bcc4306baa95000727cbcb

😎 Browse the preview: https://deploy-preview-4917--docusaurus-2.netlify.app

@github-actions
Copy link

github-actions bot commented Jun 6, 2021

⚡️ Lighthouse report for the changes in this PR:

Category Score
🟠 Performance 61
🟢 Accessibility 97
🟢 Best practices 100
🟢 SEO 100
🟢 PWA 95

Lighthouse ran on https://deploy-preview-4917--docusaurus-2.netlify.app/

@netlify
Copy link

netlify bot commented Jun 6, 2021

✔️ [V1]

🔨 Explore the source changes: 611e063

🔍 Inspect the deploy log: https://app.netlify.com/sites/docusaurus-1/deploys/60bcc43063e29f0007525a7f

😎 Browse the preview: https://deploy-preview-4917--docusaurus-1.netlify.app

@github-actions
Copy link

github-actions bot commented Jun 6, 2021

Size Change: +44 B (0%)

Total Size: 621 kB

Filename Size Change
website/build/assets/css/styles.********.css 88.3 kB +44 B (0%)
ℹ️ View Unchanged
Filename Size Change
website/build/assets/js/main.********.js 443 kB 0 B
website/build/blog/2017/12/14/introducing-docusaurus/index.html 62.3 kB 0 B
website/build/docs/introduction/index.html 235 B 0 B
website/build/index.html 27.1 kB 0 B

compressed-size-action

@lex111 lex111 force-pushed the lex111/markdown-page-layout branch from 2574391 to 611e063 Compare June 6, 2021 12:48
@slorber
Copy link
Collaborator

slorber commented Jun 9, 2021

I'm not a designer but find it better if the actual content remains centered.

I don't like too much the idea of having the TOC move the content to the left.

Also, wouldn't this lead to different md page layouts for pages with/without TOC?

Is the content of a page without TOC moved to the left (would be weird)? or in this case, the content is in the center?

I'd rather keep them all looking the same so that it's not confusing when you navigate from a page with TOC to a page without TOC (ie the content remains at the same place)

@lex111
Copy link
Contributor Author

lex111 commented Jun 9, 2021

I just implement the same behavior as on the doc page, hide_table_of_contents=true is useful if the user wants to expand the width of content part:

Default ( hide_table_of_contents=false) With hide_table_of_contents=true
image image

So I thought it would be helpful on regular pages as well.

@slorber
Copy link
Collaborator

slorber commented Jun 9, 2021

Hmmm isn't it the same screenshot? Can't tell the differences.

@yangshun any opinion regarding how the content should be centered?

@lex111
Copy link
Contributor Author

lex111 commented Jun 9, 2021

I wanted to show that if TOC is hidden, then the content column is stretched to container full width.

@slorber slorber merged commit 2913bd0 into master Jun 10, 2021
@slorber slorber deleted the lex111/markdown-page-layout branch August 17, 2021 18:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.

None yet

4 participants