Skip to content

Conversation

@nealrichardson
Copy link
Collaborator

Closes #535

@CLAassistant
Copy link

CLAassistant commented Jun 1, 2023

CLA assistant check
All committers have signed the CLA.

@wch
Copy link
Collaborator

wch commented Jun 1, 2023

Am I correct that the <meta name="viewport"> is orthogonal to the Bootstrap dependency? If that's the case, then I think it makes sense to add a tags.meta() inside of the tags.head() here:

https://github.com/rstudio/py-shiny/blob/b874f0f0b33421d17a7dd6caac888e7b989fb445/shiny/ui/_page.py#L237

@nealrichardson
Copy link
Collaborator Author

I believe you're right but I'm not confident. I just looked for where it gets added in shiny in R and tried to do the same. Happy to move it elsewhere here if you think best (and presumably do the same over there too?)

@nealrichardson
Copy link
Collaborator Author

The internet confirms that this is an HTML5 thing and not a Bootstrap thing and it should probably always be there. But the place you suggested is still inside a call to page_bootstrap()--is that right?

@wch
Copy link
Collaborator

wch commented Jun 2, 2023

But the place you suggested is still inside a call to page_bootstrap()--is that right?

That is right. Now that I've thought about it a bit more, I think it does make sense to just put it in the Bootstrap dependency like you've done, since all Bootstrap users will want it. Also, that's also how we did it on the R side and that has gone smoothly.

@nealrichardson Can you sign the CLA Assistant thing? Then feel free to merge.

@nealrichardson nealrichardson merged commit aa8326b into posit-dev:main Jun 2, 2023
@nealrichardson nealrichardson deleted the viewport branch June 2, 2023 13:39
@wch
Copy link
Collaborator

wch commented Jun 2, 2023

@schloerke I think we need to do the same for shinyswatch?

schloerke added a commit that referenced this pull request Jun 13, 2023
* main:
  chore: Replace shiny.rstudio.com with shiny.posit.co (#547)
  docs: Add badges to readme (#344)
  fix: add meta viewport tag (#540)
  fix: Use a single class for Filling Layout protocol (#530)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Missing <meta name="viewport">

3 participants