jackmoore / colorbox

A light-weight, customizable lightbox plugin for jQuery
http://www.jacklmoore.com/colorbox/
MIT License
4.75k stars 1.14k forks source link

Overlap between title and current indicator for narrow images #452

Open BradMclain opened 11 years ago

BradMclain commented 11 years ago

When an image is narrow and you have a multi image gallery the current indicator and title overlaps.

Couple suggestions:

  1. Make a minimum width that it won't shrink below that is: title width + current indicator width + close button width + next/prev buttons width)
  2. Allow user to set minium width and height
  3. Allow the current indicator text to be hidden
  4. Allow title to be displayed at the top.

Apologies if you can do any of these already but couldn't see anything so far.

jackmoore commented 11 years ago

Most of what you are describing can be done by editing the CSS. That's what was intended.

When I first wrote this plugin, I had the idea that everyone would write their own stylesheet and the examples provided now were just supposed to be examples. All those elements are just absolutely positioned and can be moved or hidden. I took the position that I wouldn't know how others would want to place them or use them. So there are definitely issues that can't be fully worked out while depending on absolutely positioned elements.

The reality is that almost nobody customized the appearance, so I would have been better served by really polishing an example or two rather than leaving it so open ended.

The 2.x branch somewhat addresses this by making it easy to change colorbox's structure. The HTML template is stored on a publicly accessible property. However I'm still not sure what the best approach will be to overflowing titles.

jackmoore commented 11 years ago

To answer your specific points, there is a minWidth/minHeight, maxWidth/maxHeight for specifying those ranges.

The title is absolutely positioned, so just move it in the CSS.

You can hide the current indicator in the CSS as well, but you have to include the important modifier so that it has higher specificity than the display set in the script. #cboxCurrent { display: none !important; }

BradMclain commented 11 years ago

Fair enough on the CSS point.

Is the minWidth only available a in the 2.x branch? couldn't find any doco on it and can't seem to get it to work?

I did find maxWidth, innerWidth, initialWidth but no minWidth.

jackmoore commented 11 years ago

Ugh. You are right, there is no minWidth/minHeight. I forgot.

johntrepreneur commented 11 years ago

I'm having the same problem and noticed that there are updates after this was posted. Does that mean this is deferred? I found a patch for this issue, but it's not ideal and says it only shows the title on hover. Any plans to fix this?

albell commented 11 years ago

I suspect the core problem is that interfaceHeight (line 381 in CB1.4.31) doesn't take any account of the title (caption) height, even though the title is visually part of the interface, and subject to variation based all on its ancestors. So all the subsequent CB sizing calculations aren't going to change when the title text is long. For long titles, and narrow viewports, where text-wrapping makes the computed height of the title taller, this is a serious layout-breaker. The caption doesn't just overlap, it disappears!

There are certainly use cases where the title would be superimposed on the image absolutely, and the author wouldn't want to include title height in interfaceHeight, so 'includeTitleInInterfaceHeightCalc` or whatever should ideally be broken out as an configurable option. Jack, thoughts? I would love to see a generalized fix for this, but don't personally understand enough of the math/workarounds further on down to venture in. The title is currently being added as a callback, so as it stands there's no obvious way without a huge refactor.

The quick solution that I use for now is just to float the caption, and pad the bottom of #cboxContent increasingly with media queries as the viewport narrows:

#cboxContent {padding: 1em 1em 7em 1em;}

@media (max-width: 32.5em) {
    #cboxContent {padding: 1em 1em 9em 1em;}
}

Yup, this is klunky. CSS "magic numbers" are kind of wrong in principle, and it takes a lot of manual testing, but it works, buying room for the absolutely positioned buttons.

bluesnag commented 8 years ago

I know this is an old post but I just modified the css for #cboxLoadedContend and #cboxTitle so that the title 'appears' on a different line and it worked for my purposes. Hope it might help someone else.

`#cboxLoadedContent{margin-bottom:50px;} / original margin-bottom was 28px /

cboxTitle{position:absolute; bottom:28px; left:0; text-align:center; width:100%; color:#949494;} / original bottom was 4px /

`