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

Create a container element for search in standalone gnav #2833

Merged
merged 11 commits into from
Sep 16, 2024
Merged

Create a container element for search in standalone gnav #2833

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
b67ae71
Adding container for for search in standalone gnav
Sep 4, 2024
13497fd
Updating class name
Sep 5, 2024
65d345a
Adding unit test
Sep 5, 2024
22ec362
Removing space to fix eslint issue
Sep 5, 2024
afa3701
Updating unit test
Sep 5, 2024
b60201c
Updating unit test
Sep 5, 2024
98cf57e
Accommodating review comments
Sep 9, 2024
24a93a0
Fixing Eslint
Sep 9, 2024
3494fbd
Accommodating review comments
Sep 9, 2024
3f52b22
Updating client search
Sep 9, 2024
5b9e6f1
Updating client search
Sep 9, 2024
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
1 change: 1 addition & 0 deletions libs/blocks/global-navigation/global-navigation.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -822,6 +822,7 @@ class Gnav {
${isDesktop.matches ? '' : this.decorateSearch()}
${this.elements.mainNav}
${isDesktop.matches ? this.decorateSearch() : ''}
${getConfig().searchEnabled === 'on' ? toFragment`<div class="feds-client-search"></div>` : ''}
Copy link
Contributor

Choose a reason for hiding this comment

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

One thing is not very clear to me. This feature is supposed to be relevant for the standalone Gnav, yet the config is added through scripts.js at a consumer level. Couldn't a config be passed directly to the standalone version? I feel like I'm missing something, but also want to make sure we're not polluting scripts.js with irrelevant properties.

Copy link
Contributor

Choose a reason for hiding this comment

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

@overmyheadandbody There is no separate config for standalone Gnav. The standalone version directly calls the init of global-navigation /global-footer and we set the miloLibs from navigation.js not through script.js.

</div>
`;

Expand Down
2 changes: 1 addition & 1 deletion libs/navigation/navigation.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ const blockConfig = [
name: 'global-navigation',
targetEl: 'header',
appendType: 'prepend',
params: ['imsClientId'],
params: ['imsClientId', 'searchEnabled'],
},
{
key: 'footer',
Expand Down
7 changes: 7 additions & 0 deletions test/blocks/global-navigation/global-navigation.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -620,4 +620,11 @@ describe('global navigation', () => {
expect(document.querySelector(`${selectors.brandImage} img`).getAttribute('src')).to.equal('http://localhost:2000/test/blocks/global-navigation/mocks/adobe-dark-logo.svg');
});
});

describe('Client search feature in global navigation', () => {
it('should append the feds-client-search div when search is enabled', async () => {
await createFullGlobalNavigation({ customConfig: { searchEnabled: 'on' } });
expect(document.querySelector(selectors.topNavWrapper).classList.contains('feds-client-search')).to.exist;
});
});
});