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-158004] Fix callout & grey line alignment issues in product merch cards #2946

Merged
merged 18 commits into from
Sep 26, 2024

Conversation

rahulgupta999
Copy link
Contributor

@rahulgupta999 rahulgupta999 commented Sep 24, 2024

I divided the body-xs slot to put the callout content between the body-xs and footer. The change was done only for the product card. The alignment strategy is the same as for mini-compare charts, where we set a minimum height for each slot.

Resolves: MWPW-158004

Test URLs:

@rahulgupta999 rahulgupta999 added bug Something isn't working run-nala Run Nala Test Automation against PR commerce merch card high priority Why is this a high priority? Blocker? Critical? Dependency? labels Sep 24, 2024
Copy link

codecov bot commented Sep 24, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.30%. Comparing base (00f139c) to head (76173ad).
Report is 5 commits behind head on stage.

Additional details and impacted files
@@            Coverage Diff             @@
##            stage    #2946      +/-   ##
==========================================
- Coverage   96.30%   96.30%   -0.01%     
==========================================
  Files         242      242              
  Lines       54873    54951      +78     
==========================================
+ Hits        52845    52918      +73     
- Misses       2028     2033       +5     

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

@Axelcureno
Copy link
Member

I don't see a difference between before and after pages. Is that expected?

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.

@rahulgupta999
Copy link
Contributor Author

rahulgupta999 commented Sep 25, 2024

I don't see a difference between before and after pages. Is that expected?

Fixed. Moved the test URL to Milo as the DC URL was not picking the branch from my fork

Copy link
Contributor

aem-code-sync bot commented Sep 25, 2024

@rahulgupta999 rahulgupta999 marked this pull request as ready for review September 25, 2024 05:59
@rahulgupta999 rahulgupta999 requested review from a team as code owners September 25, 2024 05:59
@Roycethan
Copy link

@rahulgupta999 Verified and regression covered. Observed few more issues in the latest cards tracking here since they are already exists in main: https://jira.corp.adobe.com/browse/MWPW-159140 otherwise marking "Verified"
If you are fixing https://jira.corp.adobe.com/browse/MWPW-159140 with this PR, will keep it open for you, if you are planning to fix in new PR , this PR can be labelled as Ready for stage - will allow you to review first

Also @npeltier 's approval is pending here....

cc @npeltier @afmicka

@rahulgupta999 rahulgupta999 added Ready for Main This PR is ready almost ready to be merged into main. Please inform all T1 QAs Ready for Stage and removed Ready for Main This PR is ready almost ready to be merged into main. Please inform all T1 QAs labels Sep 25, 2024
@Roycethan Roycethan added the verified PR has been E2E tested by a reviewer label Sep 25, 2024
@milo-pr-merge milo-pr-merge bot merged commit 78d5895 into adobecom:stage Sep 26, 2024
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working commerce critical high priority Why is this a high priority? Blocker? Critical? Dependency? merch card 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.

6 participants