-
Notifications
You must be signed in to change notification settings - Fork 175
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-155619: Cards overlapping sorting section- IOS (Legacy versions) #2743
MWPW-155619: Cards overlapping sorting section- IOS (Legacy versions) #2743
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## stage #2743 +/- ##
=======================================
Coverage 95.88% 95.88%
=======================================
Files 173 173
Lines 45807 45807
=======================================
Hits 43920 43920
Misses 1887 1887 ☔ View full report in Codecov by Sentry. |
5f37673
to
7a273eb
Compare
7a273eb
to
66b9011
Compare
@mirafedas Fix works on legacy versions. Need one more approval and psi-check pass , before going to stage. |
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. |
@@ -5,3 +5,4 @@ paths-ignore: | |||
- 'tools/floodgate/**' | |||
- 'tools/translation/**' | |||
- node_modules | |||
- libs/deps/mas |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this ignored? Is it enabled on the MAS side? I'd argue that anything that goes into Milo should follow its standards. If the larger community doesn't agree, then we should consider disabling it for all dependencies, not just this one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
MAS had some issues regarding lit creating code that's being flagged, which can't easily be changed / updated from what I got on a similar discussion on another PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
see here #2690 (comment) @overmyheadandbody
@mirafedas psi check is failing |
Skipped 2743: "MWPW-155619: Cards overlapping sorting section- IOS (Legacy versions)" due to file "libs/deps/mas/merch-card-collection.js" overlap. Merging will be attempted in the next batch |
On iOS version 15 and lower there is a different way of managing the vertical stacking context, which puts the menu below the merch card. Adding z-index on the
sp-action-menu
resolves this problem.Resolves: MWPW-155619
Test URLs:
Dummy page for the PSI check: https://mwpw-155619-cards-overlapping-sorting--milo--mirafedas.hlx.page/docs/