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 horizontal scroll on pages with vertical scroll #52

Closed
wants to merge 3 commits into from

Conversation

Apollinaire
Copy link

Every page that has a vertical scroll also has a small horizontal scroll.
That is because of width: 100vw; on the header. Viewport width does not care about scroll bars, so when there are vertical scrollbars the header does not resize and overflows (by 15px on Chrome).
Replacing width: 100vw; by width: 100%; fixes this issue.

@coveralls
Copy link

coveralls commented Oct 1, 2019

Pull Request Test Coverage Report for Build 181

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 56.276%

Totals Coverage Status
Change from base Build 171: 0.0%
Covered Lines: 1255
Relevant Lines: 1960

💛 - Coveralls

@electerious
Copy link
Owner

I'm using 100vw because the menu should be scrollable on mobile. It doesn't work with 100% and I'm not sure how to get it working.

Here's an example: https://codepen.io/electerious/pen/mdbNwdp (simply replace 100% with 100vwand it's working as expected)

This sounds so simple, but it seems to be very complex when using display: grid.

@Apollinaire
Copy link
Author

Thanks for the explaination. I'll look into it

@Apollinaire
Copy link
Author

Could you take a look at this?
What I did :

  • change the display on .body and #main to grid so that the header can keep scrolling without setting its width in vw
  • wrap the two overlays in a new class called overlay that has display: grid and min-height: 100vh so that the overlays are displayed in grid and centered horizontally and vertically.

On a side note, I see that you are working on the develop branch, do you want me to make the PR on this branch rather than master?

Second side note: I'm not sure if you really work using this coding style (no jsx, module.exports = , require, 'use strict' ), but it makes it fairly difficult to contribute. If you don't use this coding style and are actually generating it with another version of the code, would you mind making it public, giving dev & contribution guidelines ?

@electerious
Copy link
Owner

Thanks for the changes. I will take a look at them by time!

I somehow still can't believe that there's no simple way to limit the grid size, so I need to give it a try too.

On a side note, I see that you are working on the develop branch, do you want me to make the PR on this branch rather than master?

That's ok for now, but develop would be better in the future.

Second side note: I'm not sure if you really work using this coding style (no jsx, module.exports = , require, 'use strict' ), but it makes it fairly difficult to contribute. If you don't use this coding style and are actually generating it with another version of the code, would you mind making it public, giving dev & contribution guidelines ?

Yes, I'm writing react like this 😃 I might be the only one who likes to use the createElement (h) function instead of JSX. I will add the eslint configs.

@electerious
Copy link
Owner

electerious commented Oct 4, 2019

Adding grid-template-columns: 100%; to the grid layout seems to work: https://css-tricks.com/preventing-a-grid-blowout/

Could you give the develop branch a try?

@Apollinaire
Copy link
Author

develop looks good to me, tested on android chrome & chrome on linux desktop!

@Apollinaire Apollinaire closed this Oct 7, 2019
@electerious electerious mentioned this pull request Oct 19, 2019
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.

None yet

3 participants