-
Notifications
You must be signed in to change notification settings - Fork 1.4k
fix: mobile styles on documentation page and missing react key #3380
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
Conversation
@microsoft-github-policy-service agree |
@@ -48,6 +48,19 @@ | |||
flex-direction: row; | |||
width: 100%; | |||
|
|||
aside.handbook-toc { | |||
@media (max-width: 999px) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where did 999 come from? Are you sure this shouldn't be one of the size constants from https://github.com/microsoft/TypeScript-Website/blob/v2/packages/typescriptlang-org/src/style/globals.scss?
999 does work on my phone (yay), but I'm trying to understand if it'll work for everyone.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I used 999 because that is the breakpoint in the stylesheet where it turns from a flex row into a column. You are right though, it definitely makes more sense to use the xs breakpoint, I updated the PR to use that instead.
It would work either way because the fix is in the .whitespace.raised
style, but adding the padding just makes it look better / more readable on a mobile screen so the text doesn't go all the way to the edge.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sm, like the others here? xs is 480px, so I'm just trying to figure out what is and isn't right as (honestly) not a CSS expert
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, the media query on lines 28-30 uses the xs breakpoint to remove the padding for the content.
At 500px, the page has padding around the content:
Without the padding, when you shrink the page, the text goes all the way to the edge:
Adding the breakpoint at xs adds some padding to the page header:
Since the content is already padded at anything larger than xs, it makes more sense for that to be the breakpoint.
Azure Static Web Apps: Your stage site is ready! Visit it here: https://victorious-plant-05c166c10-3380.centralus.5.azurestaticapps.net |
fixes: #3270