-
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-159905] - Add config for full width and no border along with jarvis fix for standalone GNAV #3020
Conversation
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## stage #3020 +/- ##
==========================================
+ Coverage 96.34% 96.41% +0.07%
==========================================
Files 243 243
Lines 55284 55475 +191
==========================================
+ Hits 53261 53485 +224
+ Misses 2023 1990 -33 ☔ View full report in Codecov by Sentry. |
libs/navigation/navigation.js
Outdated
@@ -4,7 +4,7 @@ const blockConfig = [ | |||
name: 'global-navigation', | |||
targetEl: 'header', | |||
appendType: 'prepend', | |||
params: ['imsClientId', 'searchEnabled', 'unav', 'customLinks', 'jarvis'], | |||
params: ['imsClientId', 'searchEnabled', 'unav', 'customLinks', 'jarvis', 'layout'], |
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.
You don't need to pass the param layout since it is not used in the block code through milo configs.
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.
Done removed from params.
libs/navigation/bootstrapper.js
Outdated
const { getConfig, createTag, loadLink, loadScript } = await import(`${miloLibs}/utils/utils.js`); | ||
const { default: initBlock } = await import(`${miloLibs}/blocks/${name}/${name}.js`); | ||
|
||
const styles = [`${miloLibs}/blocks/${name}/${name}.css`, `${miloLibs}/navigation/navigation.css`]; | ||
styles.forEach((url) => loadLink(url, { rel: 'stylesheet' })); | ||
|
||
const setNavLayout = () => { | ||
const element = document.querySelector(targetEl); | ||
if (element && layout === 'fullWidth') { |
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.
The element will always exist, so I believe we can skip the element check here.
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.
yes correct, removed element check condition.
Reminder to set the |
Things covered as part of this PR:
layout
for supporting full width in Standalone GNAV.noBorder
for removing border for adobe home.Resolves:
MWPW-159905
MWPW-160508
Test URLs:
QA: