Multiple viewport meta tags #13230
Comments
timneutkens
added
good first issue
|
Taking a stab at this one! My apologies if I'm over-documenting my investigation process. 🙈 I don't think the ...is the problem, because farther down, after these default head elements are added to the page's other head elements, duplicates are filtered out (e.g., only the first Downloading @zvictor's example, I found that if I comment out the <Head>
{/* <meta name="viewport" content="viewport-fit=cover" /> */}
</Head>...and add it in a new export default () => (
<>
<Head>
<meta name="viewport" content="viewport-fit=cover" />
</Head>
<Viewport />
<Environment />
</>
)...it works correctly! Only the correct So the problem has to lie in |
|
Alright, in <head {...this.props}>
...
{children}
{head}
...
</head>
I'm losing the trail a bit trying to track backwards, but I think As far as how to fix this... Is this something that should only be done in next.js/packages/next/pages/_document.tsx Lines 302 to 306 in 2828b01 I'm afraid I still haven't quite wrapped my head around the difference between So which way should I move forward:
Wait a minute, though: I just realized that if I do #2, user-entered |
|
I guess I'm not the only one musing about this: #12290 |
|
Alrighty, just started a dedicated discussion #13408. Its thesis: should we discourage adding any children to |
Now I know what "render" actually means (or at least what it doesn't). 🤦🏼♂️ Learning! ✊🏼 |
|
Alright, I take from the response to #13408 that |
|
...And as per further discussion in #13408, I'm now working on a PR to warn users when they add a |
|
I would like to thank all the collaborators involved in solving this issue and releasing it in the new versions, in special @tywmick. Even though it doesn't seem that the issue was affecting him directly, he has put a lot of effort into investigating the issue and studying all possible solutions, not stopping at the first easy fix for it. Great job! 🎉 Open-source communities keep amazing me ❤️ I can confirm that I now (v9.4.5-canary.42) see the warning message in my app, as expected:
The only problem I found is that the error url is actually redirecting to a 404 destination. |
This is because the branch was not released yet, when it's on stable it will link correctly. |
|
Ok so where are we supposed to put our viewport tags? The warning without a solution is not very useful. |
@nickatrokit did you open the link attached to the warning, it says to put it in |
|
@timneutkens So where is the proper place to put site-wide metadata tags? In _document.tsx or _app.tsx? |
|
|
@chrishrtmn This seems like an odd place to put them since developers like me are used to putting them next to the html tag which is in _document. So I think most people would start by putting them in _document. |
|
For those like me who had custom components for head's children in App; I found a pattern that doesn't change much code, and is very clean => use arrays inside a wrapping function. Before :const ViewportMetaLink = () => (
<meta
name='viewport'
content='minimum-scale=1, initial-scale=1, width=device-width'
key='viewport-meta'
/>
)
const SansSerifFontLink = () => (
<link
rel='stylesheet'
href={FONTS.sansSerif.link}
key='sans-serif-font-link'
/>
)
const App = ({ Component, pageProps }: AppProps) => (
<>
<Head>
<ViewportMetaLink />
<SansSerifFontLink />
</Head>
<div />
</>
)After :const ViewportMetaLink = () => (
<meta
name='viewport'
content='minimum-scale=1, initial-scale=1, width=device-width'
key='viewport-meta'
/>
)
const SansSerifFontLink = () => (
<link
rel='stylesheet'
href={FONTS.sansSerif.link}
key='sans-serif-font-link'
/>
)
const links = () => ([ViewportMetaLink(), SansSerifFontLink()])
const App = ({ Component, pageProps }: AppProps) => (
<>
<Head>
{links()}
</Head>
<div />
</>
) |
|
So I got this warning "viewport meta tags should not be used in _document.js's " yesterday, but even after reading through this thread I do not at all understand why it is bad practice to put viewport meta tags in the in _document. For me it would be really useful if the documentation on _app and _document would be expanded to cover the differences between the two and the reasoning why we should put things like viewport metatags in either of those. Or is that documentation available and I'm just glaring over it...? |
|
Copying what I wrote on #13408 in case someone who ends up here hasn't been there:
(More discussion happening there) |
|
@Timer next.js/packages/next/next-server/lib/head.tsx Lines 118 to 141 in e6eb32f |
|
For reference to others who might experience the same issue and try to google and don't find anything --- Following the warning's advice, I moved the viewport meta tag to _app.js and wrapped it in The position, wrapped within However, I think this poses some food for thought. While I greatly appreciate that NextJS takes care of a lot things, I'm not sure it should assume anything about the presentational layer. Defaults, yes, if it makes examples easier without requiring boiler plate, that sure makes things look simple and easy! But give us the options to switch things off: Don't assume that a viewport meta tag is fine for everyone else, too. Or then establish a default that covers more ground so we can forget about managing it altogether, like (random link that covers viewport meta tag..) https://mobiforge.com/design-development/meta-viewport-best-practice |
ghost
mentioned this issue
@genox You faced this issue because you used |
@DimitarNestorov That was the problem, yes. Note to self: read properly. |
|
This issue has been automatically locked due to no recent activity. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you. |
and others
Bug report
Describe the bug
Custom viewport meta tags in the
_document.jspage are ignored by the browser because next.js pushes it's own viewport meta tag without checking if any exists already. As a result, we end up with this:I could trace the problem down to this part of the code:
https://github.com/zeit/next.js/blob/86160a5190c50ea315c7ba91d77dfb51c42bc65f/packages/next/next-server/lib/head.tsx#L13-L15
To Reproduce
_document.jspage.<meta name="viewport"I also made a full reproducible example, but I faced issues with Codesandbox. If you decide to run the reproducible, please download the source code and run it on your machine instead of the online sandbox.
Screenshots
System information
The text was updated successfully, but these errors were encountered: