Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Big images #43

Closed
marcandre opened this issue Apr 3, 2014 · 34 comments
Closed

Big images #43

marcandre opened this issue Apr 3, 2014 · 34 comments
Assignees

Comments

@marcandre
Copy link
Collaborator

Two issues with big images (i.e. images that won't fit the current window) in featherlight.

The main issue is that if the image doesn't fit width-wise, it will be resized (great!), but if it doesn't fit height-wise, it gets cut and one needs to scroll.

It makes sense for a general lightbox to allow scrolling but for images it's not really what we want (or at least what I want 😃 ).

My CSS skills aren't good enough to find a solution (either for a wiki entry or modification of the official css). I played around a bit and tried clearing width and setting both max-width and max-height to 100% but it doesn't quite work. Somehow the height is too big.

The second issue is that the behavior for IE (at least for IE 9) is different, and the image gets squished.

@noelboss can you find a solution for these?

@marcandre
Copy link
Collaborator Author

I'm really stumped by this issue 😦

@marcandre
Copy link
Collaborator Author

Posted question on StackOverflow.

@noelboss
Copy link
Owner

One possibility would be, to place the Image as the background of the container and use background-size: contain; and hide the actual image (visibility: 0), that would probably leave the possibility to still rightclick and download it and it helps keep the box wide.

@noelboss
Copy link
Owner

Checkout the development branch :) I have much improved iframe and image handling. Also, there is now no width anymore... Think should make you happy. And i fixed the background bleeding issue.

If it's okay, I'll remove the next branch, or do you have any changes there that's not yet in master?

@marcandre
Copy link
Collaborator Author

Which development branch? The master branch has changes for iframe (not related to this issue) and a z-index entry that doesn't do anything I can notice.

As for the next branch, it has not been merged, I'd like your comments about the API changes. See #39

@noelboss
Copy link
Owner

You are right, forgot to publish the development branch: https://github.com/noelboss/featherlight/tree/development

z-index should fix the buttons bleeding trough the background...

I have to check the next-branch, but probably only next week :)

@marcandre
Copy link
Collaborator Author

The development branch doesn't work either, the image gets clipped.

I posted this answer on SO:
After much twiddling around, my conclusion is that there's a fundamental difference in the way that height and width are treated in general and that it affects calculations here.

It's bound to be related to the flow of things, like how reducing the width of a <div> will have the content flow down, expanding the height, but how reducing the height of a <div> won't make it wider.

The clipping here is due to the fact that the border-bottom and padding-top are not taken into account in the available height. The solution is thus to remove those altogether.

If one still wants a border, then it can be faked by adding an absolutely positioned <div>. Here's the corresponding fiddle.

@marcandre
Copy link
Collaborator Author

I'm not sure what the default should be for featherlight and images... There's no one good solution with a frame I'm afraid.

@marcandre
Copy link
Collaborator Author

Mmm, my "solution" doesn't even work on Firefox.

@marcandre
Copy link
Collaborator Author

Resorted to using Javascript. Improved the handling of callbacks & animations to insure that image is loaded beforehand. Added a JS solution in the wiki: https://github.com/noelboss/featherlight/wiki/Resize-images-to-fit

Should we turn this into an option? It would add maybe ~16 lines of JS

@thomasloevring
Copy link

Did you ever get a solution for this?

@quasipickle
Copy link
Contributor

I've come up with an alternative, and in my humble opinion cleaner, solution.

This CSS will work in Chrome:

.featherlight .featherlight-image{
    max-width:100%;
    max-height:100%;
    width:auto;
}

but not in Firefox. The reason being, that .featherlight-content doesn't have a height set, so the max-height on the .featherlight-image uses the height of the image, not the container. See: http://stackoverflow.com/questions/20507718/percentage-max-height-on-image-ignored-in-firefox

So, my solution calculates and sets that height when the lightbox is opened, then unsets it when the lightbox closes (in case the user resizes the window):

$.featherlight.defaults.afterClose = function(){
    $(".featherlight-content").css('height','');
};

$.featherlight.defaults.afterOpen = function resizeFeatherlight(){
    var $flc = $(".featherlight-content");
    $flc.css('height',$flc.height()+'px');
}

It's a bit brute-force, but it works, and I think it's less code than @marcandre's solution (not that that code doesn't do the job - I just mention this in the interest of keeping the codebase as small as possible).

I've tested this in Firefox and Chrome. IE remains a mystery.

As for whether this functionality should be an option: I don't think it should be optional. I think it should be the default behaviour. I've never seen any other lightbox library that doesn't resize images to screen, because the whole point of a lightbox is to see the whole image.

@marcandre
Copy link
Collaborator Author

I agree this should be the default.

IE remains a mystery

Does this means it works in IE, or that you don't know if it does?

@quasipickle
Copy link
Contributor

Sorry - in retrospect that was a little cryptic.

What that means is I haven't tested it in IE.

@marcandre
Copy link
Collaborator Author

@quasipickle Sorry it took so much time before I looked at your solution. Even in Chrome, though, it doesn't quite work with the borders.

@robneu
Copy link

robneu commented Nov 27, 2014

+1 to this being the default. I also had trouble getting the example code provided here to work.

@Barzille
Copy link

Is there a (final) solution on the horizon? Will featherlight be updated to have the image fit completely into the lightbox?

@Barzille
Copy link

I have a small JS solution, which is not perfect and is a little jumpy. Simply change the afterOpen and afterClose methods to the following:

afterOpen:    function(event){
     var $flc = $(".featherlight-content");
    var featherLightImage = $flc.find('img');
    var imageRatio = featherLightImage.width() / featherLightImage.height();
        $flc.css('height',$flc.height()+'px');
        $flc.css('width',$flc.height()*imageRatio+'px');
},
afterClose:   function(event){
    $(".featherlight-content").css('height','');
    $(".featherlight-content").css('width','');
},

@MadMaxMcKinney
Copy link

Do we have a solution to this? I don't want large images to have to be scrolled.

@marcandre
Copy link
Collaborator Author

I'm actively working on a solution for this, hope to get it out in a few days.

@ghost
Copy link

ghost commented Feb 5, 2015

+1

marcandre added a commit that referenced this issue Feb 8, 2015
marcandre added a commit that referenced this issue Feb 8, 2015
@marcandre
Copy link
Collaborator Author

Merged in master, should release shortly.

@robneu
Copy link

robneu commented Feb 8, 2015

💯 x1000 👍

@elixirgraphics
Copy link

I've been experiencing scroll bars on vertical images that don't fit within the viewport as well. Using v1.2.3, which I'm assuming is the latest master branch, and still have the problem. Anyone else still having this happen as well?

@marcandre
Copy link
Collaborator Author

@elixirgraphics Probably the issue with bootstrap which I haven't checked yet. See #117 for a solution.

@elixirgraphics
Copy link

Actually it seems that when you use inline options the scroll bar comes back. If I remove the data-featherlight-close-icon inline option from my image link the problem goes away.

Scrap that. It only seemed to clear up the issue temporarily. I'll look into #117 and see if that fixes my problem.

@MickeyKay
Copy link

Any progress on this one?

@marcandre
Copy link
Collaborator Author

@MickeyKay this issue is closed...

@MickeyKay
Copy link

Right, I saw that. But the issue is not resolved and I can't find a full-on explanation of resolution elsewhere. Please see #166 for the same issue still happening.

@KodeStar
Copy link

_callbackChain onResize function (near the bottom of plugins.js line 569)

Change:

if (ratio > 1) {
    this.$content.css('width', '' + w / ratio + 'px').css('height', '' + h / ratio + 'px');
}

to

if (ratio > 1) {
    this.$content.css('width', '' + w / ratio - 50 + 'px').css('height', '' + h / ratio - 50 + 'px');
}

The 50 is to take into account the padding (25 top + 25 bottom and 25 left + 25 right)

@mweimerskirch
Copy link

Thanks @KodeStar, that fixes the issue for me as well.

@stevenbuccini
Copy link

Is this project still maintained? @KodeStar's snippet fixed the issue for me as well, and I'm surprised it hasn't been merged to master. Happy to whip up a PR if a maintainer will accept it.

cc @marcandre, @noelboss

@marcandre
Copy link
Collaborator Author

I'll definitely accept a PR, but would be best without the hardcoded "50"s, and instead getting those from the CSS somehow

@madmax2000
Copy link

madmax2000 commented Feb 23, 2017

replace

.featherlight-image {
width: auto !important;
height: auto !important;
max-width: 100%;
max-height: 90vh;
}

add overflow: hidden; to your existing code in
.featherlight .featherlight-content

These small css changes did the trick. No vertical scrollbar anymore :)

Maybe you need slightly less than 90vh
For me it was just fine

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests