accelerated-mobile-pages icon indicating copy to clipboard operation
accelerated-mobile-pages copied to clipboard

Published 20 hours ago • verified publisher icon ahmedkaludi

Migrate `amp-image-lightbox` to use `amp-lightbox-gallery`

Open Monalisha5595 opened this issue 5 years ago • 5 comments

We should use "amp-lightbox-gallery" instead of "amp-image-lightbox". In amp-lightbox-gallery some extra features are there (like,images can be zoomed and scrolled) but amp-image-lightbox does not support this.

For reference: https://github.com/ampproject/amphtml/issues/22955

https://wordpress.org/support/topic/lightbox-for-images-cant-be-zoomed-on-mobiles/

Monalisha5595 avatar Nov 14 '19 10:11 Monalisha5595

Need to remove the previous script of the lightbox - https://monosnap.com/file/Om4F5thRMRtHy4r9d4XWVIQM3q0Q2j

Zabi09 avatar Nov 25 '19 06:11 Zabi09

Lightbox true needs to be in the condition?

here https://github.com/ahmedkaludi/accelerated-mobile-pages/commit/df1e21d9306019e5bfe2260510e4a836fab87387#diff-e7f6ca8ce090f4ff7573a4eec8ccf52dR174

to here:

https://github.com/ahmedkaludi/accelerated-mobile-pages/commit/df1e21d9306019e5bfe2260510e4a836fab87387#diff-e7f6ca8ce090f4ff7573a4eec8ccf52dR188

MohammedKaludi avatar Jan 04 '20 14:01 MohammedKaludi

Ready to merge.

MohammedKaludi avatar Jan 08 '20 12:01 MohammedKaludi

This one is not solved properly. I think it needs one more review. So not merging in the deploy.

WasimM3 avatar Feb 24 '20 07:02 WasimM3

Sure.

MohammedKaludi avatar Feb 24 '20 07:02 MohammedKaludi