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

Closes #88: Add LCP Templates #116

Merged
merged 6 commits into from
Jun 20, 2024
Merged

Conversation

Miraeld
Copy link
Contributor

@Miraeld Miraeld commented Jun 19, 2024

Description

Fixes #88

Documentation

Technical documentation

This PR adds the following templates for Desktop AND Mobile:

  • lcp_bg_img_in_section
  • lcp_section_template_relative
  • lcp_section_template2
  • lcp_picture_template
  • lcp_bg_relative_attribute_incss
  • lcp_bg_img_in_header_template
  • lcp_6647_svgbg_template
  • lcp_6599_template2
  • lcp_picture_media_type_mixed
  • lcp_picture_relative

Type of change

  • Enhancement (non-breaking change which improves an existing functionality).

Checklists

Feature validation

  • I validated all the Acceptance Criteria. If possible, provide sreenshots or videos.
  • I triggered all changed lines of code at least once without new errors/warnings/notices.

@Miraeld Miraeld added the enhancement New feature or request label Jun 19, 2024
@Miraeld Miraeld requested a review from a team June 19, 2024 01:44
@Miraeld Miraeld self-assigned this Jun 19, 2024
src/support/results/expectedResultsMobile.json Outdated Show resolved Hide resolved
src/support/results/expectedResultsDesktop.json Outdated Show resolved Hide resolved
@Miraeld
Copy link
Contributor Author

Miraeld commented Jun 19, 2024

Updated to comply latest feedback :)

@Miraeld Miraeld requested a review from jeawhanlee June 20, 2024 01:07
jeawhanlee
jeawhanlee previously approved these changes Jun 20, 2024
@jeawhanlee
Copy link
Contributor

The viewport expectation for the template: lcp_picture_template2 is wrong on mobile which causes a failure for that specific template. Manual testing shows this.

Expected Viewport for mobile - /rocket-test-data/images/test3.gif for http://mike.e2e.rocketlabsqa.ovh/lcp_picture_template2 is not present in actual - [{"type":"picture","src":"http://mike.e2e.rocketlabsqa.ovh/wp-content/rocket-test-data/images/1_BDCYb3Yx8exGZVu5lLRPNw.png","sources":[{"srcset":"http://mike.e2e.rocketlabsqa.ovh/wp-content/rocket-test-data/images/lcp/testavif.avif","media":"","type":"","sizes":""}]},{"type":"picture","src":"http://mike.e2e.rocketlabsqa.ovh/test.png","sources":[]},{"type":"picture","src":"http://mike.e2e.rocketlabsqa.ovh/wp-content/rocket-test-data/images/lcp/testwebp.webp","sources":[{"srcset":"http://mike.e2e.rocketlabsqa.ovh/wp-content/rocket-test-data/images/paper.jpeg","media":"(max-width: 400px)","type":"","sizes":""},{"srcset":"http://mike.e2e.rocketlabsqa.ovh/wp-content/rocket-test-data/images/lcp/testjpeg.jpeg","media":"(max-width: 800px)","type":"","sizes":""}]}]

Expected Viewport for mobile - /rocket-test-data/images/testjpg3.jpg for http://mike.e2e.rocketlabsqa.ovh/lcp_picture_template2 is not present in actual - [{"type":"picture","src":"http://mike.e2e.rocketlabsqa.ovh/wp-content/rocket-test-data/images/1_BDCYb3Yx8exGZVu5lLRPNw.png","sources":[{"srcset":"http://mike.e2e.rocketlabsqa.ovh/wp-content/rocket-test-data/images/lcp/testavif.avif","media":"","type":"","sizes":""}]},{"type":"picture","src":"http://mike.e2e.rocketlabsqa.ovh/test.png","sources":[]},{"type":"picture","src":"http://mike.e2e.rocketlabsqa.ovh/wp-content/rocket-test-data/images/lcp/testwebp.webp","sources":[{"srcset":"http://mike.e2e.rocketlabsqa.ovh/wp-content/rocket-test-data/images/paper.jpeg","media":"(max-width: 400px)","type":"","sizes":""},{"srcset":"http://mike.e2e.rocketlabsqa.ovh/wp-content/rocket-test-data/images/lcp/testjpeg.jpeg","media":"(max-width: 800px)","type":"","sizes":""}]}]

Expected Viewport for mobile - /rocket-test-data/images/lcp/testjpg2.jpg for http://mike.e2e.rocketlabsqa.ovh/lcp_picture_template2 is not present in actual - [{"type":"picture","src":"http://mike.e2e.rocketlabsqa.ovh/wp-content/rocket-test-data/images/1_BDCYb3Yx8exGZVu5lLRPNw.png","sources":[{"srcset":"http://mike.e2e.rocketlabsqa.ovh/wp-content/rocket-test-data/images/lcp/testavif.avif","media":"","type":"","sizes":""}]},{"type":"picture","src":"http://mike.e2e.rocketlabsqa.ovh/test.png","sources":[]},{"type":"picture","src":"http://mike.e2e.rocketlabsqa.ovh/wp-content/rocket-test-data/images/lcp/testwebp.webp","sources":[{"srcset":"http://mike.e2e.rocketlabsqa.ovh/wp-content/rocket-test-data/images/paper.jpeg","media":"(max-width: 400px)","type":"","sizes":""},{"srcset":"http://mike.e2e.rocketlabsqa.ovh/wp-content/rocket-test-data/images/lcp/testjpeg.jpeg","media":"(max-width: 800px)","type":"","sizes":""}]}]

@MathieuLamiot
Copy link
Contributor

All good here on the new templates.

@MathieuLamiot MathieuLamiot merged commit 7edc223 into trunk Jun 20, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add lcp templates
4 participants