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 1 commit
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
6 changes: 6 additions & 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 @@ -813,6 +813,11 @@
analyticsValue: 'Logo',
});

decorateCustomSearch = () => {
this.elements.customSearch = toFragment`<div class="feds-search"></div>`;
return this.elements.customSearch;

Check warning on line 818 in libs/blocks/global-navigation/global-navigation.js

View check run for this annotation

Codecov / codecov/patch

libs/blocks/global-navigation/global-navigation.js#L817-L818 

Added lines #L817 - L818 were not covered by tests
};

decorateMainNav = async () => {
const breadcrumbs = isDesktop.matches ? '' : await this.decorateBreadcrumbs();
this.elements.mainNav = toFragment`<div class="feds-nav"></div>`;
Expand All @@ -822,6 +827,7 @@
${isDesktop.matches ? '' : this.decorateSearch()}
${this.elements.mainNav}
${isDesktop.matches ? this.decorateSearch() : ''}
${getConfig().searchEnabled === 'on' ? this.decorateCustomSearch() : ''}
</div>
`;

Expand Down
2 changes: 1 addition & 1 deletion libs/navigation/navigation.js
View file
Open in desktop
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ [eslint] <no-console> reported by reviewdog 🐶
Unexpected console statement.

milo/libs/navigation/navigation.js

Line 43 in b67ae71

console.error('Global navigation Error: header and footer configurations are missing.');

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't remember the context for this one, but having console errors might not be desirable. Would LANA logs not be enough?

Copy link
Contributor

Choose a reason for hiding this comment

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

This is for standalone Gnav in which we need to notify the consuming clients of the error.

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