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

✅ Add e2e test for <amp-lightbox-gallery> #22964

Merged
merged 1 commit into from Jun 24, 2019

Conversation

sparhami
Copy link

@sparhami sparhami commented Jun 21, 2019

Add a single test for now, will work on adding more tests later.

  • Change lightbox gallery manual tests to use local images instead of remote.

This simply opens/closes the gallery to make sure nothing major is
broken.
@@ -52,7 +52,7 @@ <h2>Open with expanded caption</h2>
<amp-img
on="tap:lg.open(expandDescription = true)"
lightbox="caption"
src="https://picsum.photos/300/200"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice, local images are probably a better idea

'http://localhost:8000/test/manual/amp-lightbox-gallery-launch.amp.html',
initialRect: {width: pageWidth, height: pageHeight},
// TODO(sparhami) Get this working in other environments.
environments: ['single'],
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what happens when it runs in the others? is it something to do with screen sizing or is it something else?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't dig too deeply, but when I ran locally, the page didn't show up in the viewer or shadow environments. I think it could be something about my local config though.

Copy link
Contributor

@cvializ cvializ left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@sparhami sparhami merged commit 04fd845 into ampproject:master Jun 24, 2019
thekorn pushed a commit to edelight/amphtml that referenced this pull request Sep 11, 2019
This simply opens/closes the gallery to make sure nothing major is
broken.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants