-
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-151392: Sorting button/menu issues on Mobile #2690
MWPW-151392: Sorting button/menu issues on Mobile #2690
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## stage #2690 +/- ##
=======================================
Coverage 95.91% 95.91%
=======================================
Files 173 173
Lines 45831 45834 +3
=======================================
+ Hits 43958 43961 +3
Misses 1873 1873 ☔ View full report in Codecov by Sentry. |
bbbefd0
to
061490e
Compare
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. |
4e63b9b
to
6a4efde
Compare
6a4efde
to
2240ea3
Compare
@mirafedas screenRecording-6-8-2024-12-47.mp4 |
0c6e2ce
to
6e4c54d
Compare
@mirafedas Verified and covered regression. One glitch i see hlx.live Vs hlx.page in Safari browser. where clicking on sorting button works on hlx.page (https://main--cc--adobecom.hlx.page/products/catalog?milolibs=mwpw-151392-swc-version--milo--mirafedas ) and doesn't on hlx.live ( https://main--cc--adobecom.hlx.live/products/catalog?milolibs=mwpw-151392-swc-version--milo--mirafedas ) (attached Mac Sonoma Safari browserstack video) - Not sure what could be the reason but before adding ready for stage , plz check and if you could resolve it then you can add the label on my behalf. screenRecording-13-8-2024-17-55.mp4 |
Reminder to set the |
fb5dccb
to
a9a7f5f
Compare
@Roycethan I added a fix for the .live in Safari, could you please double-check if everything works after this? |
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.
can you try await import
instead of loadScript
in merch.js
?
Also, contrary to what I said in slack, I think we should await polyfills()
in catalog before loading any swc code to make sure that custom-elements are loaded.
@yesil when I replace the |
@mirafedas awaiting the loadScript doesn't seem to block. |
@mirafedas Works now on Safari |
Skipped 2690: "MWPW-151392: Sorting button/menu issues on Mobile" due to file "libs/deps/mas/merch-card-all.js" overlap. Merging will be attempted in the next batch |
Skipped 2690: "MWPW-151392: Sorting button/menu issues on Mobile" due to file ".github/codeql/codeql-config.yml" overlap. Merging will be attempted in the next batch |
Skipped 2690: "MWPW-151392: Sorting button/menu issues on Mobile" due to file "libs/deps/mas/merch-card-all.js" overlap. Merging will be attempted in the next batch |
Skipped merging 2690: MWPW-151392: Sorting button/menu issues on Mobile due to failing checks |
Changed the SWC version to 0.45.0, which includes the fix for the action menu component.
Resolves: MWPW-151392
Test URLs:
https://mwpw-151392-swc-version--milo--mirafedas.hlx.page/ - this is for passing the PSI check. As we don't have sidenav block in Milo, only in CC, it's impossible to create a test content page at the moment