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

Set max-width to none for all images in jcrop-holder #22

Closed
wants to merge 2 commits into from

Conversation

bmaland
Copy link

@bmaland bmaland commented Jan 6, 2012

Fixes issues when combining Jcrop with responsive grids like e.g. Zurb Foundation.

Took a while to debug this, but Zurb Foundation's grid.css sets max-width to 100% for all images which completely breaks Jcrop (see screenshot below). Simply setting max-width to none for all images within the jcrop-holder fixes the issue.

Screenshot

Fixes issues when combining Jcrop with responsive grids like Zurb Foundation.
@GromNaN
Copy link
Contributor

GromNaN commented Jan 6, 2012

Since this issue is direcly related to your CSS. The fix should be in your stylesheet.

@bmaland
Copy link
Author

bmaland commented Jan 6, 2012

The fix is general though and not really specific to my stylesheet, its basically a reset which makes sure Jcrop works correctly. Frameworks like Zurb Foundation are getting quite popular and as it is everyone who wants to combine it with Jcrop will have to dig up this fix manually

@reedlaw
Copy link

reedlaw commented Feb 6, 2012

Just discovered this same issue affects Twitter Bootstrap 2.0. +1 for merge

@bmaland
Copy link
Author

bmaland commented Feb 7, 2012

Yep it will affect most sites using a responsive design, including all Zurb Foundation and Twitter Bootstrap users

@reedlaw
Copy link

reedlaw commented Feb 7, 2012

If the author feels this is an unwanted addition, at least something should go into the README or wiki. It took quite a while to diagnose the problem.

@davidmles
Copy link

+1 for merge, it fixes the problem for Twitter Bootstrap 2.0.1

@tapmodo
Copy link
Owner

tapmodo commented Apr 28, 2012

Though I do have some reservations about fixing whatever general CSS people might try to craft, I don't mind doing a few common resets since it's only a few extra bytes of CSS, and I do personally like Bootstrap a lot. I agree with @bmaland that it's general enough to warrant inclusion.

I have committed this change to the CSS and will be pushing within the hour. I am going to close this pull request. Sorry for lagging so long in getting to it, thanks for all your patience.

@tapmodo tapmodo closed this Apr 28, 2012
@equivalent
Copy link

thank you bmalad, you helped me solve this issue I was strugling for days

@tristanleboss
Copy link

I know this issue is closed for 2 years but isn't there a fix which can be applied to jcrop itself so it can deal with max-width: 100% images?

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

7 participants