thdoan / magnify

A lightweight jQuery magnifying glass zoom plugin.
https://thdoan.github.io/magnify/
MIT License
243 stars 63 forks source link

Added settings to optionally override some automatic parameters #33

Closed dfredriksen closed 7 years ago

dfredriksen commented 7 years ago

This was useful for me because my image was originally styled to have a smaller size so that a scaling animation could be applied. I needed to override the height and width of the container so that I could specify the final image size so that the magnify script would function over the proper area, so I added the extra settings.

thdoan commented 7 years ago

Thanks, I'll find some time to implement with a few corrections:

dfredriksen commented 7 years ago

Great feedback, I've posted those changes for you

thdoan commented 7 years ago

Thanks. By the way, I'm in the middle of a major update to v2.0.0 with lots of refactoring, so just a heads up that you'll see quite a few changes. I'll incorporate the intent of your PR though.

dfredriksen commented 7 years ago

Thank you, appreciate it

Sent from my Windows 10 phone

From: Tom Doan Sent: Wednesday, May 24, 2017 6:56 To: thdoan/magnify Cc: Daniel Fredriksen; Author Subject: Re: [thdoan/magnify] Added settings to optionally override someautomatic parameters (#33)

Thanks. By the way, I'm in the middle of a major update to v2.0.0 with lots of refactoring, so just a heads up that you'll see quite a few changes. I'll incorporate the intent of your PR though. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.

thdoan commented 7 years ago

When you get a chance, can you please upload or provide a link to a sample test case to demonstrate the issue you were having so that I can validate my changes?

dfredriksen commented 7 years ago

Here is a link to a similar setup: http://samples.cyint.technology/magnify/index.html

thdoan commented 7 years ago

@dfredriksen Thanks for the link. I'll push my changes tomorrow or over the weekend.

Even though I have added the override options you requested, keep in mind that for your case there is another solution that you might prefer because it would require less maintenance (you don't have to specify any additional options). This solution requires only a few simple tweaks to your code, in addition to a CSS reset that I've added to v2:

<style>
.ball {
  width: 280px; /* Use final width instead */
  animation: s1 2s ease forwards;
}
@keyframes s1 {
  from { width: 70px }
  to { width:  280px }
}
.magnify img {
  animation: none !important; /* Prevent animation reset on DOM change */
}
</style>

<script>
$(document).ready(function() {
  var $ball = $('.ball');
  // Wait for animation to end
  $ball.one('MSAnimationEnd oAnimationEnd webkitAnimationEnd animationend', function() {
    // Animation ended! Initiating plugin...
    $ball.magnify();
  });
});
</script>

See this codepen for a demo: https://codepen.io/thdoan/pen/mmoMva

dfredriksen commented 7 years ago

Thanks for that demo, that would definitely be a more effective way to implement. Appreciate your time on this matter

thdoan commented 7 years ago

Sure, no problem :). Once I release v2.0.0 (pending a few more bug fixes), you can initiate Magnify using the method above or by passing only imageWidth and imageHeight (the container will now automatically expand to fit the child elements, so no more need to specify containerWidth and containerHeight).

thdoan commented 7 years ago

Now both methods to support CSS animations are published in this demo:

https://thdoan.github.io/magnify/demo-animation.html

Note that I've renamed imageWidth and imageHeight to finalWidth and finalHeight to avoid confusion and emphasize that these options should only be used when the image has an animated lead-in. The recommended way to specify the image size for all other images remains either (1) in CSS or (2) using width and height attributes.