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

position: sticky causes sidebar overlap in Chrome #752

Closed
kvz opened this issue Jan 5, 2017 · 9 comments
Closed

position: sticky causes sidebar overlap in Chrome #752

kvz opened this issue Jan 5, 2017 · 9 comments
Assignees

Comments

@kvz
Copy link

kvz commented Jan 5, 2017

Hi, thanks for minimal-mistakes! I just deployed it yesterday on kvz.io and loving it so far. One person on reddit reported it looked weird - but it looks fine in my browsers.

The page in question is http://kvz.io/blog/2017/01/04/introducing-lanyon/ - Beyond social buttons, i did not make any layout changes, only config & markdown.

For convenience here is a direct screenshot for Chrome 56 on windows: http://i.imgur.com/BxfzHrn.png

@mmistakes
Copy link
Owner

Does the overlap happen on all pages that have the author sidebar? or just that one?

I'm on Chrome 55 on Windows and not able to reproduce it. Are there any sort of JavaScript errors in the console or is it disabled for any reason? There is a JS polyfill for position: sticky used on the sidebar that could be potentially running amok.

Other than that I'm at a loss as to what's going on.

@mmistakes
Copy link
Owner

OK, looks like it's related to position: sticky since it was added to the beta version last month. Will have to test it out and hopefully get a fix out before position: sticky support hits the stable release of Chrome... whenever that is.

@mmistakes mmistakes changed the title Chrome 56 appears to render minimal-mistakes with overlaps position: sticky causes sidebar overlap in Chrome Jan 5, 2017
@mmistakes
Copy link
Owner

Tested on beta Chrome 56 on Windows and all works as it should.

chrome_56

lanyon-grab

With position: sticky landing in Chrome stable soon seems like a good time to drop the polyfill. Wasn't adding much anyways and likely won't be missed in older browsers or IE that don't currently support it.

Still not sure why it's broken for that redditor, could be something JS related. I know there's stuff in the theme that was added awhile back that tests the width of things to trigger the sticky polyfill, and that could be borked for whatever reason.

@mmistakes mmistakes self-assigned this Jan 5, 2017
mmistakes added a commit that referenced this issue Jan 5, 2017
- Fallback to default positioning for browser that don't support it IE, Chrome < 56, etc.
- Issue #752
@kvz
Copy link
Author

kvz commented Jan 5, 2017

Blown away by your care and thoroughness in handling this issue! Humbling! ❤️

@mmistakes
Copy link
Owner

I do what I can 😉

In the end I took out the polyfill. In Firefox, Safari, and Chrome 56 the sidebar will "stick" as it should. On older browsers it scrolls with the rest of the page, which isn't so bad either.

If you're interested in making the changing it's pretty small and in this commit. I'll roll it out officially in the next release.

@mmistakes
Copy link
Owner

mmistakes commented Jan 5, 2017

You need to test on bigger screen sizes. The bug only appears when the browser is over 1150px wide and then it jumps back to working again for a little bit until the browser gets to be over 1400px wide. It appears from the GIF you posted that you didn't test on anything wider than maybe 1000px.

@nathanheffley Definitely not the case. It may be hard to tell from the GIF but the screen was much wider than 100px.

Tested on both Windows and Mac Chrome 56 (beta) and couldn't reproduce the overlap.

screen shot 2017-01-05 at 1 39 31 pm

Screen width 1875px:

screen shot 2017-01-05 at 1 39 48 pm

Curious to know if you have any plugins installed that might mess with JS or noticed any script errors in the console.

@mmistakes
Copy link
Owner

Fixed in https://github.com/mmistakes/minimal-mistakes/releases/tag/4.2.0

@kvz
Copy link
Author

kvz commented Jan 24, 2017

Getting

Could not find gem 'minimal-mistakes-jekyll (= 4.2.0)'

I should probably wait a while before it hits rubygems, sharing just in case

@mmistakes
Copy link
Owner

mmistakes commented Jan 24, 2017

@kvz Stange. It should be there, along with 4.2.1. Unless it hasn't fully propagated... https://rubygems.org/gems/minimal-mistakes-jekyll/versions/4.2.1

RobinYu added a commit to RobinYu/robinyuuu.github.io that referenced this issue Feb 13, 2017
- Fallback to default positioning for browser that don't support it IE, Chrome < 56, etc.
- Issue mmistakes#752
kkunapuli pushed a commit to kkunapuli/kkunapuli.github.io that referenced this issue May 30, 2019
- Fallback to default positioning for browser that don't support it IE, Chrome < 56, etc.
- Issue mmistakes#752
makaroniame added a commit to makaroniame/makaroniame-old.github.io that referenced this issue May 18, 2022
- Fallback to default positioning for browser that don't support it IE, Chrome < 56, etc.
- Issue mmistakes#752
jchwenger pushed a commit to jchwenger/jchwenger.github.io that referenced this issue May 5, 2023
- Fallback to default positioning for browser that don't support it IE, Chrome < 56, etc.
- Issue mmistakes#752
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants