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(v1,v2): Add initial-scale=1.0 to all meta viewport tags #3290

Merged
merged 1 commit into from
Aug 18, 2020
Merged

fix(v1,v2): Add initial-scale=1.0 to all meta viewport tags #3290

merged 1 commit into from
Aug 18, 2020

Conversation

nebrelbug
Copy link
Contributor

Motivation

When viewing my Docusaurus site (https://squirrelly.js.org) on a mobile device, it is correctly sized on first load. Almost immediately after that, however, it resizes so that it isn't the full width of the page (see below). Changing <meta name="viewport" content="width=device-width"> to <meta name="viewport" content="width=device-width, initial-scale=1.0"> solves the problem.

image

Strangely, I can't reproduce the issue on other Docusaurus sites. However, since I believe it's best practice to include initial-scale=1, I figured I'd submit the PR anyway.

Have you read the Contributing Guidelines on pull requests?

Yes

Test Plan

Visually ensure that Docusaurus doesn't change its appearance.

@nebrelbug nebrelbug requested a review from yangshun as a code owner August 16, 2020 02:31
@facebook-github-bot facebook-github-bot added the CLA Signed Signed Facebook CLA label Aug 16, 2020
@docusaurus-bot
Copy link
Contributor

Deploy preview for docusaurus-2 ready!

Built with commit 3371652

https://deploy-preview-3290--docusaurus-2.netlify.app

@slorber
Copy link
Collaborator

slorber commented Aug 17, 2020

Hi,

Not very familiar with this problem.
Can this change affect other sites in a bad way?

However, since I believe it's best practice to include initial-scale=1

What authoritative source claims that?

I think the change should be fine but try to avoid something bad by mistake :)

@@ -2,7 +2,7 @@
<html lang="en">
<head>
<meta charset="utf-8">
<meta name="viewport" content="width=device-width">
<meta name="viewport" content="width=device-width, initial-scale=1.0">
Copy link
Collaborator

Choose a reason for hiding this comment

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

btw was wondering, is this file still useful?

wasn't it you that migrated from EJS to ETA?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Haha yes it was. Actually, this file is used by https://github.com/jantimon/html-webpack-plugin, which by default uses a template engine that they call EJS but is really just lodash.template 🤷‍♂️

Maybe I should request to migrate that to Eta too 😀

@nebrelbug
Copy link
Contributor Author

Can this change affect other sites in a bad way?

I can't say for positive, but I'm pretty sure it won't. I guess the best way is just to test the Docusaurus v2 site? The deploy preview for this PR looks good to me.

What authoritative source claims that?

Here are a few I found after a quick Google Search:

Hopefully this PR will just fix a few edge cases like mine without negatively impacting anyone

@slorber slorber added the pr: bug fix This PR fixes a bug in a past release. label Aug 18, 2020
@slorber slorber changed the title Add initial-scale=1.0 to all meta viewport tags fix(v2): Add initial-scale=1.0 to all meta viewport tags Aug 18, 2020
@slorber slorber changed the title fix(v2): Add initial-scale=1.0 to all meta viewport tags fix(v1,v2): Add initial-scale=1.0 to all meta viewport tags Aug 18, 2020
@slorber
Copy link
Collaborator

slorber commented Aug 18, 2020

So, this seems fine, let's merge, but if people complain I'll likely revert

@slorber slorber merged commit a94d14b into facebook:master Aug 18, 2020
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.

4 participants