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

MWPW-154026: Long CTAs fall in the second line in merch card footer #2565

Merged
merged 7 commits into from
Jul 23, 2024

Conversation

mirafedas
Copy link
Contributor

@mirafedas mirafedas commented Jul 12, 2024

If the CTA text is long and there is a secure transaction label displayed, the CTA extends beyond the card because the secure transaction label has its height set to 100%. Removing this property does not break the footer elements' alignment in any card variation.
I added a block of four cards with long CTA text for testing.

Corresponding Mas PR (has to merged before I can build it from main): adobecom/mas#38

Resolves: MWPW-154026

Before:
Screenshot 2024-07-12 at 13 46 59

After:
Screenshot 2024-07-12 at 13 47 26

Test URLs:
Before:

After:

@mirafedas mirafedas changed the title updated merch-card with no height set for secure transaction MWPW-154026: Long CTAs fall in the second line in merch card footer Jul 12, 2024
Copy link

codecov bot commented Jul 12, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.67%. Comparing base (99b33a0) to head (c3dbccf).
Report is 1 commits behind head on stage.

Additional details and impacted files
@@           Coverage Diff           @@
##            stage    #2565   +/-   ##
=======================================
  Coverage   95.67%   95.67%           
=======================================
  Files         170      170           
  Lines       45302    45302           
=======================================
  Hits        43342    43342           
  Misses       1960     1960           

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

Copy link
Contributor

@3ch023 3ch023 left a comment

Choose a reason for hiding this comment

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

approving, once QA will verify and we'll merge mas PR we should update the file though :)

@3ch023 3ch023 added run-nala Run Nala Test Automation against PR commerce labels Jul 12, 2024
Copy link
Contributor

aem-code-sync bot commented Jul 12, 2024

Page Scores Audits Google
/drafts/mirafedas/mwpw-154026-long-cta?martech=off PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI

@Roycethan Roycethan added the verified PR has been E2E tested by a reviewer label Jul 13, 2024
@Roycethan
Copy link

@mirafedas verified

Copy link
Contributor

Reminder to set the Ready for Stage label - to queue this to get merged to stage & production.

@@ -1,7 +1,7 @@
// branch: main commit: 258ad3d851cc4945eae2aab2dcc80a8d0c14d861 Fri, 28 Jun 2024 14:19:21 GMT
import{html as a,LitElement as j}from"/libs/deps/lit-all.min.js";import{LitElement as q,html as T,css as F}from"/libs/deps/lit-all.min.js";var s=class extends q{static properties={size:{type:String,attribute:!0},src:{type:String,attribute:!0},alt:{type:String,attribute:!0},href:{type:String,attribute:!0}};constructor(){super(),this.size="m",this.alt=""}render(){let{href:e}=this;return e?T`<a href="${e}">
// branch: MWPW-154026-cta-long-text commit: b9a46be0956fd4df485c08a14cb59a0e51814c78 Fri, 12 Jul 2024 14:20:01 GMT
Copy link
Contributor

Choose a reason for hiding this comment

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

@npeltier , @yesil , @3ch023 - what's the correct way to build this dependency? I've seen it being built from master, develop, some feature branches along with multiple comments saying it should be built from develop or from a feature branch. Can you please shed some light here? Thanks!

Copy link
Member

Choose a reason for hiding this comment

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

It should be built from main

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.

@skumar09 skumar09 added run-nala Run Nala Test Automation against PR and removed run-nala Run Nala Test Automation against PR labels Jul 18, 2024
@narcis-radu
Copy link
Contributor

@mirafedas - I think you need to build from main now and @Roycethan will have to add 'Ready for Stage' label

@mirafedas
Copy link
Contributor Author

@narcis-radu will do it as soon as the corresponding mas PR is merged: adobecom/mas#38 . I added this link in the PR description just now and will do the same in upcoming PRs to have a quick reference. Thanks for checking! :)

@milo-pr-merge milo-pr-merge bot merged commit 2c5b9a4 into adobecom:stage Jul 23, 2024
16 checks passed
@milo-pr-merge milo-pr-merge bot mentioned this pull request Jul 23, 2024
rohitsahu pushed a commit to rohitsahu/milo that referenced this pull request Jul 26, 2024
* stage:
  MWPW-154998 [MEP][MILO] Manifests do not execute in the right order when there is a disabled manifest (adobecom#2632)
  mwpw-154965: Fetch federal stage content from hlx.page instead of stage.adobe.com (adobecom#2618)
  Correct error messages for duplicate files on the stage to main workflow (adobecom#2621)
  MWPW-153245 [merch][analytics] dispatch wcomp events, and let default lh (adobecom#2610)
  Revert "MWPW-146528[MILO][MEP][ANALYTICS] Add attribute to content changed by Target for analytics and MWPW-152274" (adobecom#2627)
  MWPW-128600 Locale Tool: Langstore points to langstore/en (adobecom#2615)
  Fix for errors in dynamically loaded scripts in test cases (adobecom#2619)
  MWPW-146528[MILO][MEP][ANALYTICS] Add attribute to content changed by Target for analytics and MWPW-152274 (adobecom#2593)
  Bootstrapper script for milo feds blocks (adobecom#2560)
  Revert "[MWPW-152968] mWeb - Passing ECID to Branch.io banner - Implementation" (adobecom#2612)
  MWPW-146743 Improve Article Header Performance (adobecom#2577)
  MWPW-153808: fix duplicate tax label (adobecom#2614)
  MWPW-154026: Long CTAs fall in the second line in merch card footer (adobecom#2565)
  Revert "[MWPW-154795] Style Feds Global-footer region picker drop-up variant (without hash)" (adobecom#2611)
  [AUTOMATED-PR] Update imslib.min.js dependency (adobecom#2605)
  [MWPW-154795] Style Feds Global-footer region picker drop-up variant (without hash) (adobecom#2599)
  MWPW-143053 [MEP] Request for New Personalization Tag - CC Paid (adobecom#2604)
  [MWPW-152674] [Gray Box] Desktop gnav not hidden when device view is open (adobecom#2597)
  MWPW-150566 - 🆕 Editorial-Card block (adobecom#2533)

# Conflicts:
#	libs/deps/merch-card.js
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
commerce Ready for Stage run-nala Run Nala Test Automation against PR verified PR has been E2E tested by a reviewer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants