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

Add search button to header #3483

Open
wants to merge 29 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 23 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
b021a18
add initial changes
precious-onyenaucheya-ons Jan 29, 2025
7bf2beb
update styling and approve visual test
precious-onyenaucheya-ons Jan 30, 2025
4d81fc4
limit popular searches
precious-onyenaucheya-ons Jan 30, 2025
b0eb10b
rename class
precious-onyenaucheya-ons Jan 30, 2025
586e897
remove redundant styling
precious-onyenaucheya-ons Jan 30, 2025
672fa08
Merge branch 'main' into feature/162-header-search-button
precious-onyenaucheya-ons Feb 6, 2025
6b43a75
update styling
precious-onyenaucheya-ons Feb 6, 2025
b375a29
update onslogo size
precious-onyenaucheya-ons Feb 7, 2025
7fb0698
more styling update
precious-onyenaucheya-ons Feb 10, 2025
289b12b
Merge branch 'main' into feature/162-header-search-button
precious-onyenaucheya-ons Feb 12, 2025
700de62
update the search button
precious-onyenaucheya-ons Feb 12, 2025
f6c0fef
fix tests
precious-onyenaucheya-ons Feb 17, 2025
5378d07
approve visual tests
precious-onyenaucheya-ons Feb 17, 2025
4cc084c
fix toggle
precious-onyenaucheya-ons Feb 18, 2025
b1bf012
Merge branch 'main' into feature/162-header-search-button
precious-onyenaucheya-ons Feb 21, 2025
b3b5c22
revert changes to the ons icon
precious-onyenaucheya-ons Feb 21, 2025
82fc26d
update styling and visual tests
precious-onyenaucheya-ons Feb 21, 2025
8335a2e
address PR comments
precious-onyenaucheya-ons Feb 21, 2025
0a4cfe4
Merge branch 'main' into feature/162-header-search-button
precious-onyenaucheya-ons Feb 24, 2025
446da04
address pr comments
precious-onyenaucheya-ons Feb 26, 2025
3df9796
update focus state
precious-onyenaucheya-ons Feb 27, 2025
f56ccec
remove unecessary style
precious-onyenaucheya-ons Mar 4, 2025
6cd046d
Fix issue with breakpoints
precious-onyenaucheya-ons Mar 4, 2025
69c7998
remove important key word in css
precious-onyenaucheya-ons Mar 5, 2025
7b2e368
update close icon svg and fix focus states
precious-onyenaucheya-ons Mar 6, 2025
26eb83c
remove extra space in macro file
precious-onyenaucheya-ons Mar 6, 2025
a4bd79b
address comments on the search
precious-onyenaucheya-ons Mar 7, 2025
809dbdc
fix more issues with the menu and search
precious-onyenaucheya-ons Mar 7, 2025
9150216
Merge branch 'main' into feature/162-header-search-button
precious-onyenaucheya-ons Mar 13, 2025
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
20 changes: 20 additions & 0 deletions __snapshots__/layout/_template.spec.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -230,10 +230,14 @@ exports[`base page template matches the favicons block override snapshot 1`] = `

</div>



</div>
</div>





</div>

Expand Down Expand Up @@ -473,10 +477,14 @@ exports[`base page template matches the footer block override snapshot 1`] = `

</div>



</div>
</div>





</div>

Expand Down Expand Up @@ -848,6 +856,7 @@ exports[`base page template matches the full configuration snapshot 1`] = `

</div>



</div>
</div>
Expand Down Expand Up @@ -890,6 +899,9 @@ exports[`base page template matches the full configuration snapshot 1`] = `

</nav>





</div>

Expand Down Expand Up @@ -2005,10 +2017,14 @@ exports[`base page template matches the meta block override snapshot 1`] = `

</div>



</div>
</div>





</div>

Expand Down Expand Up @@ -2232,10 +2248,14 @@ exports[`base page template matches the social block override snapshot 1`] = `

</div>



</div>
</div>





</div>

Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Copy link
Contributor

Choose a reason for hiding this comment

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

Im not sure why this has changed. It looks like the radios at the bottom with the dropdown have changed slightly

Copy link
Contributor

Choose a reason for hiding this comment

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

@precious-onyenaucheya-ons have you looked in to this one? Doesn't look major but curious to see if there is a reason for the change

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@admilne @rmccar I am not sure why this changed, I have not made any change in this ticket that will affect this pattern.

Copy link
Contributor

Choose a reason for hiding this comment

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

This one needs investigating

Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
151 changes: 150 additions & 1 deletion src/components/button/_button.scss
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,46 @@ $button-shadow-size: 3px;
}
}

&--header-search & {
&__inner {
background: var(--ons-color-ocean-blue);
}
}

&--header-search &,
&--header-search:active &,
&--header-search.active & {
&__inner {
padding: 0.875rem !important;
Copy link
Contributor

@rmccar rmccar Feb 21, 2025

Choose a reason for hiding this comment

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

This doesn't seem to need important. If we can we should avoid using important and only use it if we need to. Can you review the places youve added important and check that they are needed

Copy link
Contributor

Choose a reason for hiding this comment

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

@rmccar I see this hasn't changed. It looks like it is required at the moment because there is some css later on in the file which overrides the padding needed here. On line 780 in _button.scss you have the following:

.ons-search__btn {
    height: 92.5%; //this is to allow the button to be fully aligned with the input by accounting for the shadow box of 3px
    .ons-btn__inner:has(> .ons-icon) {
        padding-right: 0.75rem;
        height: 100%;
    }
}

This looks a little odd to me, anyone know if this is required? But this is the style resulting in @precious-onyenaucheya-ons needing to use !important. It's more specific and comes later in the file

Copy link
Contributor

Choose a reason for hiding this comment

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

This code was to align the height of the button when it is used in places like this:
https://service-manual.ons.gov.uk/design-system/components/input/example-input-search

Before this the button shadow would sit below the input.

This seems to be fine when important is removed. Have you seen any issues?

Copy link
Contributor

Choose a reason for hiding this comment

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

@rmccar yeah, it's subtle but the button isn't quite as wide without the important tag. I can't remember off the top of my head but I think the icon perhaps wasn't quite centre either?

border: 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this needed? This line seems to make no difference

Copy link
Contributor

Choose a reason for hiding this comment

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

@precious-onyenaucheya-ons I don't think this comment has been addressed yet?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed

Copy link
Contributor

Choose a reason for hiding this comment

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

@precious-onyenaucheya-ons I think this is still in the code?

Copy link
Contributor

Choose a reason for hiding this comment

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

This doesn't look like its been removed to me either

border-radius: 0;
box-shadow: none;
.ons-icon {
fill: var(--ons-color-text-inverse) !important;
height: 28px;
width: 28px;
}
}
}

&--header-search:focus &,
&--header-search:hover &,
&--header-search:active &,
&--header-search.active &,
&--header-search:active:focus &,
&--header-search.active:focus & {
&__inner {
background: var(--ons-color-night-blue) !important;
}
}

&--header-search:focus & {
&__inner {
z-index: 4;
outline: 4px solid var(--ons-color-focus);
}
}

&__inner {
background: var(--ons-color-button);
border-radius: $input-radius;
Expand Down Expand Up @@ -556,7 +596,7 @@ $button-shadow-size: 3px;
&--menu {
align-items: center;
display: flex;
padding: 1.5rem;
padding: 1rem;
border-bottom: 4px solid rgb(0 0 0 / 0%);
.ons-icon {
transform: rotate(90deg);
Expand All @@ -575,6 +615,9 @@ $button-shadow-size: 3px;
margin-top: 0;
width: 1rem;
}
.ons-btn__text {
height: 24px;
}
}
}

Expand Down Expand Up @@ -629,6 +672,112 @@ $button-shadow-size: 3px;
}
}
}

&--search-icon,
&--close {
border-bottom: 4px solid transparent;
align-items: center;
display: flex;
padding: 1rem;
}

&--search-icon &,
&--close & {
&__inner {
background: rgb(0 0 0 / 0%);
box-shadow: none;
padding: 0;
.ons-icon {
@extend .ons-u-m-no;

fill: var(--ons-color-text-link);
height: 1.5rem;
width: 1.5rem;
}
}
}

&--search-icon:hover {
border-bottom: 4px solid rgb(0 0 0 / 0%);
border-color: var(--ons-color-text-link-hover);
}

&--search-icon:focus:hover:not(:active, .active) {
.ons-btn__inner {
background: none;
box-shadow: none;
}
}

&--close:focus:hover:not(:active, .active) {
.ons-btn__inner {
background: none;
box-shadow: none;
}
}

&--close:active,
&--close[aria-expanded='true'] {
@extend .ons-u-ml-no;

border-bottom: 4px solid transparent;
background-color: var(--ons-color-branded-tint);
}

&--search-icon:hover &,
&--search-icon:active &,
&--search-icon:active:focus &,
&--search-icon.active &,
&--search-icon.active:focus & {
&__inner {
background: none;
box-shadow: none;
color: var(--ons-color-text-link-hover);
.ons-icon {
fill: var(--ons-color-text-link-hover);
}
}
}

&--search-icon:focus {
background-color: var(--ons-color-focus);
border-color: var(--ons-color-text-link-focus);
}

&--search-icon:focus &,
&--close:focus & {
&__inner {
background: none;
box-shadow: none;
.ons-icon {
fill: var(--ons-color-text-link-focus);
}
}
}

&--close:focus {
background: var(--ons-color-focus);
}

&--search-icon:active {
background-color: var(--ons-color-branded-tint);
border-color: var(--ons-color-text-link-hover);
}

&--close:hover &,
&--close:active &,
&--close.active &,
&--close.active:focus &,
&--close:active:focus & {
&__inner {
background: none !important;
box-shadow: none;
color: var(--ons-color-text-link-hover);
.ons-icon {
fill: var(--ons-color-text-link-hover);
}
}
}
}

.ons-search__btn {
Expand Down
4 changes: 4 additions & 0 deletions src/components/button/_macro.njk
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,10 @@
{% set iconType = "loader" %}
{% set iconPosition = "after" %}
{% set variantClasses = " ons-btn--loader ons-js-loader ons-js-submit-btn" %}
{% elif 'search' in params.variants %}
{% set iconType = "search" %}
{% elif 'close' in params.variants %}
{% set iconType = "close" %}
{% endif %}
{% endif %}
{% set tag = "a" if params.url else "button" %}
Expand Down
20 changes: 18 additions & 2 deletions src/components/header/_header.scss
Original file line number Diff line number Diff line change
Expand Up @@ -168,13 +168,13 @@
}

&__org-logo--large {
@include mq('2xs', 454px) {
@include mq('2xs', 460px) {
display: none;
}
}

&__org-logo--small {
@include mq(455px) {
@include mq(461px) {
display: none;
}
}
Expand Down Expand Up @@ -278,6 +278,22 @@
}
}

&-nav-search {
background-color: var(--ons-color-branded-tint);
margin-bottom: 1rem;
@extend .ons-u-pt-2xl;
@extend .ons-u-pb-2xl;

width: 100%;
&__input {
border-bottom: 1px solid var(--ons-color-ocean-blue);
@extend .ons-u-mb-2xl;
@extend .ons-u-pb-2xl;

row-gap: 1rem;
}
}

.ons-btn {
top: 0 !important;
}
Expand Down
23 changes: 21 additions & 2 deletions src/components/header/_macro-options.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
| navigation | array`<Navigation>` | false | Settings for the [main menu links](#navigation) |
| siteSearchAutosuggest | `Autosuggest` [_(ref)_](/components/autosuggest) | false | Sets the autosuggest functionality in the header |
| menuLinks | object`<MenuLinks>` | false | Settings for the [menu button navigation](#menuLinks) in the masthead |
| searchLinks | object`<SearchLinks>` | false | Settings for the [search button navigation](#searchLinks) in the masthead |

## MastheadLogo

Expand Down Expand Up @@ -71,9 +72,27 @@
| ariaLabel | string | false | The `aria-label` attribute added to the `<nav>` element. Defaults to “Menu links navigation”. |
| ariaListLabel | string | false | The `aria-label` attribute added to the `<ul>` element. Defaults to “Menu links”. |
| keyLinks | array`<KeyLink>` | true | Settings for an array of [key list items](#keylink) |
| columns | array`<Column>` | true | Settings for list of [columns](#column) that contain menu links |
| columns | array`<Column>` | true | Settings for list of [columns](#column) that contain menu links |
| toggleNavButton | object`<ToggleButton>` | true | Settings for the [menu toggle button](#toggleButton) |

## SearchLinks

| Name | Type | Required | Description |
| ------------------ | ---------------------- | -------- | ----------------------------------------------------------------------------------------------------------------------------------------------- |
| id | string | true | The HTML `id` of the `search button` element. Used for the `aria-controls` attribute for the search toggle button displayed on small viewports. |
| classes | string | false | Classes to add to the `search button` element |
| ariaLabel | string | false | The `aria-label` attribute added to the `search button` element. Defaults to Search navigation”. |
| toggleSearchButton | object`<ToggleButton>` | false | Settings for the [search toggle button](#toggleButton) |
| heading | string | true | The heading label for the search items list |
| itemsList | array`<SearchItem>` | true | Settings for an array of [searches](#searchitem) associated with each search link. The list can contain a maximum of 5 items. |

## SearchItem

| Name | Type | Required | Description |
| ---- | ------ | -------- | ----------------------------- |
| text | string | true | The text for the search item. |
| url | string | true | The URL for the search item |

## Language

| Name | Type | Required | Description |
Expand Down Expand Up @@ -161,7 +180,7 @@
| ---------- | ------------------ | -------- | --------------------------------------------------------------------- |
| heading | string | true | The heading label for the menu group |
| url | string | false | The URL for the HTML `href` attribute for the path to the linked page |
| groupItems | array`<groupItem>` | false | Settings for an array of [group items](#groupitem) for each list item |
| groupItems | array`<groupItem>` | false | Settings for an array of [group items](#groupitem) for each list item |

## GroupItem

Expand Down
Loading