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-152041[MEP][MILO] Add support for using MEP placeholders in gnav #2446

Merged
merged 1 commit into from
Jun 11, 2024
Merged

MWPW-152041[MEP][MILO] Add support for using MEP placeholders in gnav #2446

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion libs/blocks/global-navigation/utilities/utilities.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,14 +147,15 @@ let fedsPlaceholderConfig;
export const getFedsPlaceholderConfig = ({ useCache = true } = {}) => {
if (useCache && fedsPlaceholderConfig) return fedsPlaceholderConfig;

const { locale } = getConfig();
const { locale, placeholders } = getConfig();
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this tie into another PR? I'm not getting how placeholders end up in the config, it's not obvious from its definition.

Copy link
Contributor

Choose a reason for hiding this comment

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

Didn't check the whole flow, but I would assume this is part of the personalization logic

Copy link
Contributor

Choose a reason for hiding this comment

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

Could these be just the manifest ones? 🤔 This line could hint at that. Better to get a quick explanation. Code is straightforward enough, but I'd like to understand that part for my own knowledge.

Copy link
Contributor

Choose a reason for hiding this comment

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

@overmyheadandbody and @mokimo , I actually went through and cleaned up mep related items in config, and placeholders go overlooked. Since this PR is a blocker, I don't want to hold up merging it for this housekeeping item. So I made separate ticket for the follow up work. But I think having the placeholders data nested inside config.mep will help clear up this confusion.

Ticket for followup work: https://jira.corp.adobe.com/browse/MWPW-152192

const libOrigin = getFederatedContentRoot();

fedsPlaceholderConfig = {
locale: {
...locale,
contentRoot: `${libOrigin}${locale.prefix}/federal/globalnav`,
},
placeholders,
};

return fedsPlaceholderConfig;
Expand Down
Loading