Remove transitions for cropped and object-fit contain images from Lightbox #13040
Conversation
cathyxz
force-pushed
the
bugfix/cropped-images
branch
from
40b8579
to
8da28b8
Compare
| @@ -208,7 +209,7 @@ export class AmpImageViewer extends AMP.BaseElement { | |||
|
|
|||
| /** | |||
| * Returns the boundaries of the image element. | |||
| * @return {!Element} | |||
| * @return {?Element} | |||
There was a problem hiding this comment.
Is it nullable? Looks like it asserts.
There was a problem hiding this comment.
The intention was to remove the assertElement. Oops.
There was a problem hiding this comment.
The reasoning here is that we're technically not guaranteed that this.image_ will be initialized until we return from layout callback. So there's the option between either returning a promise here and forcing the lightbox exit to wait for this to layout, or just to check in lightbox exit and skip the animation if no image has been laid out yet. Since the entire exit animation is dependent on the image actually loading, it is pointless to wait for this image to layout for just an exit animation.
There was a problem hiding this comment.
Cool, that makes a lot of sense
| } else { | ||
| const img = elementByTag(dev().assertElement(this.sourceAmpImage_), | ||
| 'img'); | ||
| if (img) { |
There was a problem hiding this comment.
nit: In the rest of the codebase we simplify if statements like this to ternaries.
| const elementHeight = ampImage./*OK*/offsetHeight; | ||
| const elementWidth = ampImage./*OK*/offsetWidth; | ||
| const ampImageAspectRatio = elementWidth / elementHeight; | ||
| return Math.abs(naturalAspectRatio - ampImageAspectRatio) > 0.001; |
…htbox (ampproject#13040) * Get natural image width * No transition for images whose aspect ratios have changed * Address PR comments
…htbox (ampproject#13040) * Get natural image width * No transition for images whose aspect ratios have changed * Address PR comments
…htbox (ampproject#13040) * Get natural image width * No transition for images whose aspect ratios have changed * Address PR comments
Take these out because they look broken. Next step, actually implement a reasonable animation for them.