8000 Revert "Prevent a console error from the global navigation main menu" by salonijain3 · Pull Request #3629 · adobecom/milo · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Revert "Prevent a console error from the global navigation main menu" #3629

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
Merged
merged 1 commit into from
Feb 5, 2025
Merged

Revert "Prevent a console error from the global navigation main menu" #3629

merged 1 commit into from
Feb 5, 2025

Conversation

salonijain3
Copy link
Contributor
@salonijain3 salonijain3 commented Feb 5, 2025 •
edited by sharmrj
Loading

@aem-code-sync AEM Code Sync
Copy link
Contributor
aem-code-sync bot commented Feb 5, 2025 •
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

@spadmasa
Copy link
spadmasa commented Feb 5, 2025

Validated the revert pr
https://main--homepage--adobecom.hlx.page/homepage/index-loggedout?milolibs=revert-3625-svg-url
https://main--cc--adobecom.hlx.page/creativecloud/animation/discover?milolibs=revert-3625-svg-url
image

@spadmasa spadmasa added verified PR has been E2E tested by a reviewer Ready for Stage high priority Why is this a high priority? Blocker? Critical? Dependency? high-impact Any PR that may affect consumers labels Feb 5, 2025
@overmyheadandbody
Copy link
Contributor

PSI checks are not passing, you should provide test URLs. What are the remediation steps to address this issue moving forward, since this PR reverts the changes? The console error will continue to be visible on prod.

@mokimo
Copy link
Contributor
mokimo commented Feb 5, 2025 •
edited
Loading

I agree with Rares, we should still address the initial issue & add some test links in the revert PR the next time. However those are issues to address after this initial revert.

@mokimo mokimo merged commit bbbd2b5 into stage Feb 5, 2025
26 of 27 checks passed
@mokimo mokimo deleted the revert-3625-svg-url branch February 5, 2025 17:02
@mokimo mokimo mentioned this pull request Feb 5, 2025
Merged
@aem-code-sync AEM Code Sync
Copy link
Contributor
aem-code-sync bot commented Feb 5, 2025
Page Scores Audits Google
📱 /?martech=off PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI
🖥️ /?martech=off PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI

@sharmrj
Copy link
Contributor
sharmrj commented Feb 5, 2025

PSI checks are not passing, you should provide test URLs. What are the remediation steps to address this issue moving forward, since this PR reverts the changes? The console error will continue to be visible on prod.

@overmyheadandbody Turns out the svg was entirely unnecessary. #3630

8000
colloyd pushed a commit to colloyd/milo that referenced this pull request Feb 6, 2025
@salonijain3 @colloyd
Revert "Prevent a console error from the global navigation main menu" (…
33edb67 
…adobecom#3629)

Revert "Prevent a console error from the global navigation main menu (adobecom#3625)"

This reverts commit 91fb8d4.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mokimo mokimo mokimo approved these changes

@sharmrj sharmrj sharmrj approved these changes

@sonawanesnehal3 sonawanesnehal3 sonawanesnehal3 approved these changes

@bandana147 bandana147 Awaiting requested review from bandana147

@nishantka nishantka Awaiting requested review from nishantka

@Deva309 Deva309 Awaiting requested review from Deva309

Assignees
No one assigned
Labels
high priority Why is this a high priority? Blocker? Critical? Dependency? high-impact Any PR that may affect consumers Ready for Stage verified PR has been E2E tested by a reviewer
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@salonijain3 @spadmasa @overmyheadandbody @mokimo @sharmrj @sonawanesnehal3
0