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

If original image has a transition, zoom becomes uncloseable #110

Closed
birjj opened this issue Jul 31, 2019 · 6 comments · Fixed by #148 or docsifyjs/docsify#1050
Closed

If original image has a transition, zoom becomes uncloseable #110

birjj opened this issue Jul 31, 2019 · 6 comments · Fixed by #148 or docsifyjs/docsify#1050

Comments

@birjj
Copy link

birjj commented Jul 31, 2019

Bug description

If the image used for the zoom has a transition applied in its style attribute, the zoom will happen (but without animating the transform of the zoom image) but will be uncloseable since medium-zoom never sets isAnimating = false.

This is because medium-zoom listens for the transitionend event, with no fallback. Since the target element is copied to create the zoom image, the style attribute is copied too - and this is never overwritten (the transition defined by medium-zoom is done so in a stylesheet, and is therefore overwritten by the style). Since this means that there's no transition: transform, the transition never starts or ends, and no transitionend event is emitted.

How to reproduce

  1. Create an image with style="transition: opacity 0.5s" or similar
  2. Run medium-zoom(...) on your image
  3. Click the image - the zoom will open instantly and become uncloseable

Expected behavior

It works even if image has a transition set in its style.

Reproducible example

Link to the bug reproduction

@birjj
Copy link
Author

birjj commented Aug 1, 2019

@francoischalifour I am fairly certain this is a bug, not an enhancement ;)

@francoischalifour
Copy link
Owner

Thanks for the report.

Depends on the way you see it haha. It's a case that is not handled, but the library works as expected on more common use cases.

Anyway, supporting this will probably make the code more complex and add some bytes to the final bundle. Feel free to open a PR to see this case supported so we can discuss about it being part of a next release.

@birjj
Copy link
Author

birjj commented Aug 1, 2019

@francoischalifour There are generally two ways to fix this: you can either add !important to the transition you define in your stylesheet (in which case it still breaks if the user adds a transition: ... !important, but y'know, that's pretty edge case), or you can explicitly set the transition on the cloned element e.g. active.zoomed.style.transition = "transform 300ms cubic-bezier(0.2, 0, 0.2, 1)";

Neither should add much complexity.

@madeleineostoja
Copy link

I'm also seeing this, in my case with a 3rd party library (gatsby-image) where I have no control over the transitions defined. Either one of the patches mentioned by @birjolaxew would be great, even as opt-in behaviour

@francoischalifour
Copy link
Owner

Thank you all for your interest in this issue.

I rather lean toward the CSS fix with !important because people could already be relying on the CSS classes. Adding the transition in JavaScript could be a breaking change in some cases.

You can expect this issue to be fixed in v1.0.5.

Meanwhile, you can override the medium-zoom-image CSS class in your apps so that it doesn't collide with other transitions.

.medium-zoom-image {
  transition: transform 300ms cubic-bezier(0.2, 0, 0.2, 1) !important;
}

francoischalifour added a commit that referenced this issue Dec 7, 2019
**Description**

This makes sure that the opening/closing transitions are triggered even though an image CSS `transition` is provided as inline style.

This is problematic with frameworks that generate inline styles on their
images (e.g. Gatsby).

See full context in #110.

**Solution**

The CSS `transition` is marked as "!important" for the animation to happen
even though it's overriden by another inline `transition` style attribute.

**Related**

Closes #110
francoischalifour added a commit that referenced this issue Dec 7, 2019
**Description**

This makes sure that the opening/closing transitions are triggered even though an image CSS `transition` is provided as inline style.

This is problematic with frameworks that generate inline styles on their
images (e.g. Gatsby).

See full context in #110.

**Solution**

The CSS `transition` is marked as "!important" for the animation to happen
even though it's overriden by another inline `transition` style attribute.

**Related**

Closes #110
francoischalifour added a commit that referenced this issue Dec 7, 2019
This makes sure that the opening/closing transitions are triggered even though an image CSS `transition` is provided as inline style.

This is problematic with frameworks that generate inline styles on their
images (e.g. Gatsby).

Closes #110
@francoischalifour
Copy link
Owner

Released in 1.0.5 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants