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-140627] [MEP] Replace MEP placeholders when loading fragments #3193

Merged
merged 2 commits into from
Nov 14, 2024

Conversation

AdobeLinhart
Copy link
Contributor

@AdobeLinhart AdobeLinhart commented Nov 13, 2024

When we insert text/HTML in MEP, we call the replacePlaceholders function to replace any already known placeholders (aka MEP placeholders).  We want to do the same thing when it's fragment content that has a MEP placeholder.  So we'll want to call that function again from fragments.js

At the end of fragments.js init function, we are calling handleMepCommands. Afterwards, I think we should test for config.placeholders and fragment's html having a placeholder.  If so, run it through replacePlaceholders.

Note, since there were now 3 if statements that all repeated importing personalization.js for 3 different functions, they were moved to all be nested inside 1 if.

QA instructions
The main element on the test page only uses MEP placeholders, but the gnav also uses regular placeholders. To test this PR, check the performance tab in dev tools for the before and after links. Not only should the LCP be much faster, but it should not call placeholders.json until after LCP in the after link.

Resolves: MWPW-140627

Test URLs:

Copy link
Contributor

aem-code-sync bot commented Nov 13, 2024

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

Copy link
Contributor

aem-code-sync bot commented Nov 13, 2024

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

Copy link

codecov bot commented Nov 13, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.36%. Comparing base (1fa592a) to head (98c87a3).
Report is 579 commits behind head on stage.

Additional details and impacted files
@@            Coverage Diff             @@
##            stage    #3193      +/-   ##
==========================================
+ Coverage   95.69%   96.36%   +0.67%     
==========================================
  Files         165      245      +80     
  Lines       43542    56610   +13068     
==========================================
+ Hits        41668    54554   +12886     
- Misses       1874     2056     +182     

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

@vgoodric vgoodric requested a review from a team November 13, 2024 21:53
Copy link
Contributor

Reminder to set the Ready for Stage label - to queue this to get merged to stage & production.

@vgoodric vgoodric added verified PR has been E2E tested by a reviewer Ready for Stage labels Nov 14, 2024
@milo-pr-merge milo-pr-merge bot merged commit 0e407ec into stage Nov 14, 2024
25 checks passed
@milo-pr-merge milo-pr-merge bot deleted the fragmentplaceholders branch November 14, 2024 17:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Ready for Stage verified PR has been E2E tested by a reviewer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants