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

Wrapper should be optional #14

Open
augnustin opened this issue Jan 31, 2019 · 3 comments
Open

Wrapper should be optional #14

augnustin opened this issue Jan 31, 2019 · 3 comments

Comments

@augnustin
Copy link

Hey there,

Great plugin, exactly what I am looking for. 😄

I am slightly disappointed though, because I expected to simply have to write:

MediumLightbox('.article-content img');

in my articles and it would work, but it doesn't.

I happen to write my articles in markdown, so I don't have control on the HTML wrapping logic. Also data-width and data-height are great, but by default, those could value the current width and height of the image.

Finally the content of style.css is unclear: is it code for your example or is it library code? The distinction should be clear. And eventually you might want to consider moving the CSS code to a JS file, so that the plugin becomes much more agnostic, with single file dependency.

Thanks for taking those feedback in consideration

@augnustin
Copy link
Author

Regarding the dimensions, I wrote this mini jQuery function that calculates the natural dimensions of the image:

$.fn.addNaturalDimensions = function() {
  this.filter('img').each(function() {
    var $img = $(this);
    var newImg = new Image();
    newImg.onload = function() {
      $img.attr('data-width', newImg.width);
      $img.attr('data-height', newImg.height);
    }
    newImg.src = $img.attr('src');
  });
  return this;
}

Using jQuery but would be really simple to remove it.

@augnustin
Copy link
Author

Regarding the wrapper issue, I had to wrap it at two levels:

$('.article-content img')
  .wrap('<div class="img-wrapper"><div class="sub-wrapper"></div></div>');

MediumLightbox('.article-content .img-wrapper');

@augnustin
Copy link
Author

Also the zoomed image looked blury because the translate was not an integer number of pixels.

Changed:

			//wrap coordinates
			var wrapX = parseInt(((screenSize.x-scrollbarWidth)/2)-imgL - (imgW/2));
			var wrapY = parseInt(imgT*(-1) + (screenSize.y-imgH)/2);

I think this lib needs a good refreshing!

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

No branches or pull requests

1 participant