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.

Sign up for GitHub

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

Jump to bottom

MWPW-157005 Marketo thank you fixes #2779

Merged
merged 2 commits into from
Sep 2, 2024
Merged

MWPW-157005 Marketo thank you fixes #2779

merged 2 commits into from
Sep 2, 2024

Conversation

Brandon32
Copy link
Contributor

@Brandon32 Brandon32 commented Aug 22, 2024 •
edited
Loading

@Brandon32 Brandon32 added run-nala Run Nala Test Automation against PR @marketo Run Marketo Block Tests labels Aug 22, 2024
@aem-code-sync AEM Code Sync
Copy link
Contributor

aem-code-sync bot commented Aug 22, 2024 •
edited
Loading

Hello, I'm the AEM Code Sync Bot and I will run some actions to deploy your branch and validate page speed.
In case there are problems, just click a checkbox below to rerun the respective action.

  • Re-run PSI checks
  • Re-sync branch
Commits

@aem-code-sync AEM Code Sync
Copy link
Contributor

aem-code-sync bot commented Aug 22, 2024 •
edited
Loading

Page Scores Audits Google
M /drafts/bmarshal/one-page/one-page-video?martech=off PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI
D /drafts/bmarshal/one-page/one-page-video?martech=off PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI

@codecov Codecov
Copy link

codecov bot commented Aug 22, 2024 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.92%. Comparing base (d90dcc0) to head (0ad152b).
Report is 17 commits behind head on stage.

Additional details and impacted files 
@@            Coverage Diff             @@
##            stage    #2779      +/-   ##
==========================================
+ Coverage   95.90%   95.92%   +0.01%     
==========================================
  Files         173      173              
  Lines       45842    45857      +15     
==========================================
+ Hits        43967    43988      +21     
+ Misses       1875     1869       -6

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@aem-code-sync aem-code-sync bot temporarily deployed to bmarshal/thank-you August 22, 2024 18:09 Inactive
@aem-code-sync aem-code-sync bot temporarily deployed to bmarshal/thank-you August 22, 2024 18:35 Inactive
@aem-code-sync aem-code-sync bot temporarily deployed to bmarshal/thank-you August 22, 2024 18:41 Inactive
@github-actions GitHub Actions
Copy link
Contributor

This pull request is not passing all required checks. Please see this discussion for information on how to get all checks passing. Inconsistent checks can be manually retried. If a test absolutely can not pass for a good reason, please add a comment with an explanation to the PR.

@aem-code-sync aem-code-sync bot temporarily deployed to bmarshal/thank-you August 23, 2024 15:14 Inactive
@Brandon32 Brandon32 force-pushed the bmarshal/thank-you branch 2 times, most recently from b656dfe to e20253b Compare August 23, 2024 15:37
@aem-code-sync aem-code-sync bot temporarily deployed to bmarshal/thank-you August 23, 2024 15:37 Inactive
JasonHowellSlavin
JasonHowellSlavin approved these changes Aug 23, 2024
View reviewed changes
libs/blocks/marketo/marketo.css Outdated
column-gap: 4.6%;
align-items: flex-start;
.marketo form.mktoForm.mktoForm--fade-in.mktoVisible {
max-height: 10000px;
Copy link
Contributor

Choose a reason for hiding this comment

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

thinking about it, we may want to store this and 400px as variables for readability into why these numbers were chosen?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good call, we can set a more accurate number than 400px, 291px for 3 rows or 369px for 4 rows.

  --marketo-form-placeholder-height: calc(78px * 3 + 57px); /* 3 rows + submit */

@JasonHowellSlavin JasonHowellSlavin added the verified PR has been E2E tested by a reviewer label Aug 26, 2024
@Brandon32
Copy link
Contributor Author

This pull request is not passing all required checks. Please see this discussion for information on how to get all checks passing. Inconsistent checks can be manually retried. If a test absolutely can not pass for a good reason, please add a comment with an explanation to the PR.

PSI check failing due to above the fold form with third party dependency.

@milo-pr-merge
Copy link
Contributor

milo-pr-merge bot commented Aug 31, 2024

Skipped merging 2779: MWPW-157005 Marketo thank you fixes due to failing checks

2 similar comments
@milo-pr-merge
Copy link
Contributor

milo-pr-merge bot commented Sep 1, 2024

Skipped merging 2779: MWPW-157005 Marketo thank you fixes due to failing checks

@milo-pr-merge
Copy link
Contributor

milo-pr-merge bot commented Sep 2, 2024

Skipped merging 2779: MWPW-157005 Marketo thank you fixes due to failing checks

@mokimo mokimo merged commit 582c244 into stage Sep 2, 2024
19 of 20 checks passed
@mokimo mokimo deleted the bmarshal/thank-you branch September 2, 2024 08:21
This was referenced Sep 2, 2024
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@robert-bogos robert-bogos robert-bogos approved these changes

@JasonHowellSlavin JasonHowellSlavin JasonHowellSlavin approved these changes

@overmyheadandbody overmyheadandbody Awaiting requested review from overmyheadandbody

@mokimo mokimo Awaiting requested review from mokimo

@meganthecoder meganthecoder Awaiting requested review from meganthecoder meganthecoder is a code owner

Assignees

@JasonHowellSlavin JasonHowellSlavin

Labels
@marketo Run Marketo Block Tests Ready for Stage run-nala Run Nala Test Automation against PR verified PR has been E2E tested by a reviewer
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@Brandon32 @JasonHowellSlavin @robert-bogos @Dli3 @mokimo