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.

Sign up for GitHub

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

Jump to bottom

MWPW-158537 : Upgrade Universal Nav to version 1.4 #3367

Open
wants to merge 7 commits into
base: stage
Choose a base branch
from
Open

MWPW-158537 : Upgrade Universal Nav to version 1.4 #3367

wants to merge 7 commits into from
+53 −11

Conversation

sonawanesnehal3
Copy link
Contributor

Upgrade Universal Nav to version 1.4

  • Add messageEventListener to avoid onMessage event listener (Ref)
  • Support Profile Switching in Account Menu (Ref1, Ref2)
  • Support new Mobile UX designs for Unav (Ref)
  • Enable shopping cart (Ref)

Resolves: MWPW-158537

Test URLs:

@sonawanesnehal3 sonawanesnehal3 added do not merge PR should not be merged yet needs-verification PR requires E2E testing by a reviewer labels Dec 12, 2024
@aem-code-sync AEM Code Sync
Copy link
Contributor

aem-code-sync bot commented Dec 12, 2024 •
edited
Loading

Hello, I'm the AEM Code Sync Bot and I will run some actions to deploy your branch and validate page speed.
In case there are problems, just click a checkbox below to rerun the respective action.

  • Re-run PSI checks
  • Re-sync branch
Commits

@sonawanesnehal3 sonawanesnehal3 removed the do not merge PR should not be merged yet label Dec 12, 2024
@aem-code-sync AEM Code Sync
Copy link
Contributor

aem-code-sync bot commented Dec 12, 2024 •
edited
Loading

Page Scores Audits Google
📱 /drafts/snehal/illustrator?martech=off PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI
🖥️ /drafts/snehal/illustrator?martech=off PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI

@codecov Codecov
Copy link

codecov bot commented Dec 12, 2024 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.47%. Comparing base (a2443c0) to head (e8603e1).
Report is 19 commits behind head on stage.

Additional details and impacted files 
@@           Coverage Diff           @@
##            stage    #3367   +/-   ##
=======================================
  Coverage   96.47%   96.47%           
=======================================
  Files         255      255           
  Lines       59253    59265   +12     
=======================================
+ Hits        57163    57176   +13     
+ Misses       2090     2089    -1

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

@sonawanesnehal3 sonawanesnehal3 changed the title Upgrade Universal Nav to version 1.4 MWPW-158537 : Upgrade Universal Nav to version 1.4 Dec 12, 2024
@sonawanesnehal3 sonawanesnehal3 added needs-verification PR requires E2E testing by a reviewer and removed needs-verification PR requires E2E testing by a reviewer labels Dec 12, 2024
@github-actions GitHub Actions
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.

@github-actions GitHub Actions
Copy link
Contributor

This PR has not been updated recently and will be closed in 7 days if no action is taken. Please ensure all checks are passing, https://github.com/orgs/adobecom/discussions/997 provides instructions. If the PR is ready to be merged, please mark it with the "Ready for Stage" label.

@github-actions github-actions bot added Stale and removed Stale labels Dec 26, 2024
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Jan 9, 2025

This PR is currently in the needs-verification state. Please assign a QA engineer to verify the changes.

narcis-radu
narcis-radu reviewed Jan 10, 2025
View reviewed changes
libs/blocks/global-navigation/global-navigation.js
@@ -74,17 +74,27 @@ export const CONFIG = {
name: 'profile',
attributes: {
isSignUpRequired: false,
messageEventListener: (event) => {
const { name, payload, executeDefaultAction } = event.detail;
if (name === 'System' && payload.subType === 'AppInitiated') {
Copy link
Contributor

Choose a reason for hiding this comment

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

We can fail fast here

if (name !== 'System') return;
switch (payload.subType) {
    case 'AppInitiated':
        // code
    case 'SignOut':
        // code
    default:
        // code
}

libs/blocks/global-navigation/global-navigation.js
executeDefaultAction();
}
if (name === 'System' && payload.subType === 'ProfileSwitch') {
executeDefaultAction().then((profile) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is executeDefaultAction always a Promise?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@narcis-radu narcis-radu narcis-radu left review comments

@sharmrj sharmrj sharmrj approved these changes

@bandana147 bandana147 bandana147 approved these changes

@salonijain3 salonijain3 Awaiting requested review from salonijain3 salonijain3 is a code owner

@nishantka nishantka Awaiting requested review from nishantka nishantka is a code owner

@Deva309 Deva309 Awaiting requested review from Deva309 Deva309 is a code owner

Assignees

@spadmasa spadmasa

@prativas22 prativas22

Labels
needs-verification PR requires E2E testing by a reviewer
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@sonawanesnehal3 @narcis-radu @bandana147 @sharmrj @spadmasa @prativas22