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-142003: Mini Compare Chart Mobile styling #1887

Merged
merged 25 commits into from
Mar 9, 2024

Conversation

Axelcureno
Copy link
Member

@Axelcureno Axelcureno commented Feb 16, 2024

Introduces mobile styles for Mini Compare Chart card and an event listener for tab button clicks in order to trigger resize the cards not showing in the DOM.

Screenshot 2024-02-13 at 12 31 12 PM
Screenshot 2024-02-13 at 12 31 02 PM

Tacocat PR-related: https://git.corp.adobe.com/wcms/tacocat.js/pull/538

Resolves: MWPW-142003

Test URLs:

@Axelcureno Axelcureno added bug Something isn't working run-nala Run Nala Test Automation against PR commerce needs-verification PR requires E2E testing by a reviewer merch card labels Feb 16, 2024
@Axelcureno Axelcureno self-assigned this Feb 16, 2024
@Axelcureno Axelcureno requested a review from a team as a code owner February 16, 2024 06:35
Copy link
Contributor

@yesil yesil left a comment

Choose a reason for hiding this comment

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

@Axelcureno

I think, we could achieve this as follows at the line 404.

const container = el.closest('main > div');
or
const container = document.querySelector('main');. (main one should be done only once)
Please try that first, I would like us to avoid event listeners as much as possible.

Copy link

codecov bot commented Feb 16, 2024

Codecov Report

Attention: Patch coverage is 83.33333% with 4 lines in your changes are missing coverage. Please review.

Project coverage is 96.29%. Comparing base (f00900b) to head (e50e2d1).

Files Patch % Lines
libs/blocks/merch-card/merch-card.js 86.95% 3 Missing ⚠️
libs/blocks/merch-card/merch-offer-select.js 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##            stage    #1887      +/-   ##
==========================================
- Coverage   96.29%   96.29%   -0.01%     
==========================================
  Files         165      165              
  Lines       42285    42301      +16     
==========================================
+ Hits        40718    40733      +15     
- Misses       1567     1568       +1     

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

@Axelcureno
Copy link
Member Author

const container = el.closest('main > div');

@yesil I agree we should avoid event listeners. Unfortunately, I tried both approaches and none worked.

@Axelcureno Axelcureno requested a review from Roycethan February 22, 2024 20:27
@Axelcureno
Copy link
Member Author

@Roycethan can you please verify this one?

@Roycethan
Copy link

Roycethan commented Feb 22, 2024

@Roycethan can you please verify this one?

@Axelcureno Plz review these issues:

  1. Most popular label is overlapped:
image

2)Not sure if this device is in scope but just adding here for this resolution - left/right broders:
image

  1. Theres no enough bottom padding at the bottom of last card - is it author-able ?

@Blainegunn Blainegunn changed the base branch from main to stage March 1, 2024 22:55
Copy link
Contributor

aem-code-sync bot commented Mar 5, 2024

Page Scores Audits Google
/drafts/axel/block-samples/fragments/mini-compare-chart PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI

Axelcureno and others added 8 commits March 5, 2024 10:51
Co-Authored-By: ilyas Stéphane Türkben <isturkben@gmail.com>
Co-Authored-By: ilyas Stéphane Türkben <isturkben@gmail.com>
Co-Authored-By: ilyas Stéphane Türkben <isturkben@gmail.com>
@Axelcureno Axelcureno merged commit dbc5de6 into adobecom:stage Mar 9, 2024
9 of 11 checks passed
yesil pushed a commit to yesil/milo that referenced this pull request Mar 11, 2024
* MWPW-142003: Mini Compare Chart Mobile styling
---------
Co-authored-by: ilyas Stéphane Türkben <isturkben@gmail.com>
yesil added a commit that referenced this pull request Mar 11, 2024
yesil added a commit that referenced this pull request Mar 11, 2024
Revert "MWPW-142003: Mini Compare Chart Mobile styling (#1887)"

This reverts commit dbc5de6.
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 merch card needs-verification PR requires E2E testing by a reviewer run-nala Run Nala Test Automation against PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants