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

Allow content to scroll #27

Closed
wants to merge 2 commits into from
Closed

Allow content to scroll #27

wants to merge 2 commits into from

Conversation

mmanela
Copy link
Contributor

@mmanela mmanela commented May 6, 2015

When I tested the plugin I was only able to see the top part of it in both chrome and firefox. I was not able to scroll to the rest.

The following changes attempt to fix that by allowing it to scroll.

@micmro
Copy link
Owner

micmro commented May 7, 2015

Thanks a lot for the contribution. I have just a few questions:
I assume this bug happened on a site that hides the vertical overflow? Making it scrollable will probably cause issues with regular pages, did you test if that worked?
If not can you give me a few example URL that failed so I can test it as well?

@mmanela
Copy link
Contributor Author

mmanela commented May 7, 2015

I noticed the issue jsfiddle.net. I did test on other scrollable sites and it works ok. This plugin just becomes a 100% scrollable pane

@micmro
Copy link
Owner

micmro commented May 8, 2015

I finally got around to build your branch and test it. It looks good, the only concern I have (sorry to be nit-picky) is that it won't allow you on regular (non overflow:hidden) pages to take a full length screenshot. I think this could be solved in code though: if overflow:hidden is set on the body use your style else use the existing one.
Unfortunately I probably won't have time to look into this until the weekend, but would be happy if you can give it a stab.

@micmro micmro mentioned this pull request May 8, 2015
@micmro
Copy link
Owner

micmro commented May 13, 2015

Merged with PR #30

@micmro micmro closed this May 13, 2015
@mmanela
Copy link
Contributor Author

mmanela commented May 13, 2015

Thanks!

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

2 participants