[docs] Home: fix layout on mobile with slow connection #19283
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jonsamp
approved these changes
Sep 29, 2022
There was a problem hiding this comment.
Thanks for addressing this. I tried to reproduce this bug, but was not able to. To test this branch, I:
- Cloned your branch
- Made a production build and ran it
- Throttled my network to Slow 3G
- Then navigated from the website to the docs
The result looked fine, so I am happy with shipping this. Just left one code style comment that I would like to fix, but functionality looks good to me.
Comment on lines
+19
to
+27
| const CustomCol = ({ children, sm, md, lg, xl, xxl }: PropsWithChildren<ColProps>) => ( | ||
| <> | ||
| <Col css={cellWrapperStyle} sm={sm} md={md} lg={lg} xl={xl} xxl={xxl}> | ||
| {children} | ||
| </Col> | ||
| <div css={mobileCellWrapperStyle}>{children}</div> | ||
| </> | ||
| ); | ||
|
|
There was a problem hiding this comment.
We should put this in its own file (one file = one component)
There was a problem hiding this comment.
This file includes all variation of Cells (multiple components) already, and CustomCol is not exported - it has been only used by those cell components, so I think that moving it to it's own file isn't a great idea in this particular case.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Why
Hopefully fixes https://twitter.com/tejas89_patel/status/1574819679947829248
How
The grid component that we are using relay on the JS to inject the correct breakpoint styles, which on slow connection on mobile can lead to the broken layout when loading.
We can alter the default initial screen size setting, but this would lead to the issues with slow connection on desktop.
An alternate solution to that is to switch to the static containers on mobile, utilizing media queries.
Test Plan
The docs apps has been build and served locally, then I have attempted to load it in browser with Throttle settings set to "Slow 3G".
Preview
Locally served docs
https://docs.expo.dev/
Checklist
expo prebuild& EAS Build (eg: updated a module plugin).