Skip to content

Commit f64092c

Browse files
committed
Refactor ds-search-navbar so the <form> is only rendered when expanded
Pa11y/HTMLCS rule WCAG2AA H32.2 ("This form does not contain a submit button") fires on every page because the navbar search renders a <form> even while collapsed, and the icon control inside it is type="button" (it has to be: in collapsed state its job is to expand the input, not to submit). Functionally, Enter still submits the search via HTML5 implicit submission on a single-input form, so this is a false positive in behavioural terms. The rule is, however, pointing at a real semantic ambiguity: the icon button's role depends on whether the search is collapsed or expanded. Restructure the template so: - when collapsed, only a single <button type="button"> is rendered (the magnifying-glass toggle). No form is in the DOM. - when expanded, the full <form> is rendered with <input> and a real <button type="submit"> as the icon. Enter submits natively; clicking the icon submits natively. The icon's role is now unambiguous in each state and Pa11y H32.2 no longer reports the navbar form. The cypress e2e tests (`cypress/e2e/search-navbar.cy.ts`) keep working because they query the icon by `[data-test="header-search-icon"]`, which is set on both the collapsed-state toggle and the expanded-state submit button. The expand-on-collapse slide animation defined by `expandSearchInput` is still applied to the input element while expanded; it is no longer applied to the entry transition (the form simply appears). A follow-up can re-introduce a `:enter`/`:leave` slide on the conditional form if desired. Fixes #5603
1 parent 9b025a6 commit f64092c

3 files changed

Lines changed: 80 additions & 32 deletions

File tree

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,24 @@
11
<div [title]="'nav.search' | translate" (dsClickOutside)="collapse()">
22
<div class="d-inline-block position-relative">
3-
<form [formGroup]="searchForm" (ngSubmit)="onSubmit(searchForm.value)" autocomplete="on" class="d-flex">
4-
<input #searchInput [@toggleAnimation]="isExpanded" [attr.aria-label]="('nav.search' | translate)" name="query"
5-
formControlName="query" type="text" placeholder="{{searchExpanded ? ('nav.search' | translate) : ''}}"
6-
class="d-inline-block bg-transparent position-absolute form-control dropdown-menu-end p1"
7-
[class.display]="searchExpanded ? 'inline-block' : 'none'"
8-
[tabIndex]="searchExpanded ? 0 : -1"
9-
[attr.data-test]="'header-search-box' | dsBrowserOnly">
10-
<button class="submit-icon btn btn-link btn-link-inline" [attr.aria-label]="'nav.search.button' | translate" type="button" (click)="searchExpanded ? onSubmit(searchForm.value) : expand()" [attr.data-test]="'header-search-icon' | dsBrowserOnly" tabindex="0" role="button">
3+
@if (!searchExpanded) {
4+
<button type="button" class="search-toggle btn btn-link btn-link-inline"
5+
[attr.aria-label]="'nav.search.button' | translate"
6+
(click)="expand()"
7+
[attr.data-test]="'header-search-icon' | dsBrowserOnly">
118
<em class="fas fa-search fa-lg fa-fw"></em>
129
</button>
13-
</form>
10+
} @else {
11+
<form [formGroup]="searchForm" (ngSubmit)="onSubmit(searchForm.value)" autocomplete="on" class="d-flex">
12+
<input #searchInput [@toggleAnimation]="isExpanded" [attr.aria-label]="('nav.search' | translate)" name="query"
13+
formControlName="query" type="text" placeholder="{{'nav.search' | translate}}"
14+
class="d-inline-block bg-transparent position-absolute form-control dropdown-menu-end p1"
15+
[attr.data-test]="'header-search-box' | dsBrowserOnly">
16+
<button type="submit" class="submit-icon btn btn-link btn-link-inline"
17+
[attr.aria-label]="'nav.search.button' | translate"
18+
[attr.data-test]="'header-search-icon' | dsBrowserOnly">
19+
<em class="fas fa-search fa-lg fa-fw"></em>
20+
</button>
21+
</form>
22+
}
1423
</div>
1524
</div>

src/app/search-navbar/search-navbar.component.spec.ts

Lines changed: 28 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -70,13 +70,24 @@ describe('SearchNavbarComponent', () => {
7070
expect(component).toBeTruthy();
7171
});
7272

73-
describe('when you click on search icon', () => {
73+
describe('when collapsed', () => {
74+
it('should render only the toggle <button> with no surrounding <form>', () => {
75+
const form = fixture.debugElement.query(By.css('form'));
76+
const toggle = fixture.debugElement.query(By.css('button.search-toggle'));
77+
expect(form).toBeNull();
78+
expect(toggle).not.toBeNull();
79+
expect(toggle.nativeElement.tagName).toBe('BUTTON');
80+
expect(toggle.nativeElement.getAttribute('type')).toBe('button');
81+
});
82+
});
83+
84+
describe('when you click on the search toggle', () => {
7485
beforeEach(fakeAsync(() => {
7586
spyOn(component, 'expand').and.callThrough();
7687
spyOn(component, 'onSubmit').and.callThrough();
7788
spyOn(router, 'navigate');
78-
const searchIcon = fixture.debugElement.query(By.css('form .submit-icon'));
79-
searchIcon.triggerEventHandler('click', {
89+
const toggle = fixture.debugElement.query(By.css('button.search-toggle'));
90+
toggle.triggerEventHandler('click', {
8091
preventDefault: () => {/**/
8192
},
8293
});
@@ -86,16 +97,22 @@ describe('SearchNavbarComponent', () => {
8697

8798
it('input expands', () => {
8899
expect(component.expand).toHaveBeenCalled();
100+
expect(component.searchExpanded).toBeTrue();
101+
});
102+
103+
it('renders the form with a real submit button', () => {
104+
const form = fixture.debugElement.query(By.css('form'));
105+
const submit = fixture.debugElement.query(By.css('form button.submit-icon'));
106+
expect(form).not.toBeNull();
107+
expect(submit).not.toBeNull();
108+
expect(submit.nativeElement.getAttribute('type')).toBe('submit');
89109
});
90110

91111
describe('empty query', () => {
92112
describe('press submit button', () => {
93113
beforeEach(fakeAsync(() => {
94-
const searchIcon = fixture.debugElement.query(By.css('form .submit-icon'));
95-
searchIcon.triggerEventHandler('click', {
96-
preventDefault: () => {/**/
97-
},
98-
});
114+
const form = fixture.debugElement.query(By.css('form'));
115+
form.triggerEventHandler('submit', null);
99116
tick();
100117
fixture.detectChanges();
101118
}));
@@ -117,17 +134,16 @@ describe('SearchNavbarComponent', () => {
117134
searchInput.nativeElement.dispatchEvent(new Event('input'));
118135
fixture.detectChanges();
119136
});
120-
describe('press submit button', () => {
137+
describe('press Enter on the input (native submit)', () => {
121138
beforeEach(fakeAsync(() => {
122-
const searchIcon = fixture.debugElement.query(By.css('form .submit-icon'));
123-
searchIcon.triggerEventHandler('click', null);
139+
const form = fixture.debugElement.query(By.css('form'));
140+
form.triggerEventHandler('submit', null);
124141
tick();
125142
fixture.detectChanges();
126143
}));
127144
it('to search page with query', async () => {
128145
const extras: NavigationExtras = { queryParams: { query: 'test' } };
129146
expect(component.onSubmit).toHaveBeenCalledWith({ query: 'test' });
130-
131147
expect(router.navigate).toHaveBeenCalledWith(['search'], extras);
132148
});
133149
});

src/app/search-navbar/search-navbar.component.ts

Lines changed: 34 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
11
import {
2+
AfterViewInit,
3+
ChangeDetectorRef,
24
Component,
35
ElementRef,
46
ViewChild,
@@ -17,7 +19,13 @@ import { BrowserOnlyPipe } from '../shared/utils/browser-only.pipe';
1719
import { ClickOutsideDirective } from '../shared/utils/click-outside.directive';
1820

1921
/**
20-
* The search box in the header that expands on focus and collapses on focus out
22+
* The search box in the header that expands on focus and collapses on focus out.
23+
*
24+
* When collapsed, only a single icon `<button>` is rendered (no form). The form, input
25+
* and submit button are only rendered while the search bar is expanded; this avoids the
26+
* Pa11y/HTMLCS rule WCAG2AA H32.2 ("This form does not contain a submit button"), which
27+
* is otherwise reported on every page because the collapsed-state form had a
28+
* `type="button"` icon control rather than a real submit button.
2129
*/
2230
@Component({
2331
selector: 'ds-base-search-navbar',
@@ -32,57 +40,72 @@ import { ClickOutsideDirective } from '../shared/utils/click-outside.directive';
3240
TranslateModule,
3341
],
3442
})
35-
export class SearchNavbarComponent {
43+
export class SearchNavbarComponent implements AfterViewInit {
3644

3745
// The search form
3846
searchForm;
3947
// Whether or not the search bar is expanded, boolean for html ngIf, string for AngularAnimation state change
4048
searchExpanded = false;
4149
isExpanded = 'collapsed';
4250

43-
// Search input field
51+
// Search input field. Only present once the form is rendered (`searchExpanded === true`).
4452
@ViewChild('searchInput') searchField: ElementRef;
4553

46-
constructor(private formBuilder: UntypedFormBuilder, private router: Router, private searchService: SearchService) {
54+
constructor(
55+
private formBuilder: UntypedFormBuilder,
56+
private router: Router,
57+
private searchService: SearchService,
58+
private cdr: ChangeDetectorRef,
59+
) {
4760
this.searchForm = this.formBuilder.group(({
4861
query: '',
4962
}));
5063
}
5164

65+
ngAfterViewInit(): void {
66+
// No-op; here so the @ViewChild reference is wired up after each render pass when
67+
// the form is conditionally added/removed from the DOM.
68+
}
69+
5270
/**
53-
* Expands search bar by angular animation, see expandSearchInput
71+
* Expands the search bar and focuses the input on the next change-detection pass
72+
* (the input only exists once `searchExpanded` is true).
5473
*/
5574
expand() {
5675
this.searchExpanded = true;
5776
this.isExpanded = 'expanded';
77+
// Force a synchronous render so `searchField` is populated, then focus it.
78+
this.cdr.detectChanges();
5879
this.editSearch();
5980
}
6081

6182
/**
62-
* Collapses & blurs search bar by angular animation, see expandSearchInput
83+
* Collapses & blurs the search bar. The form, input and submit button are removed
84+
* from the DOM by the template's `@if (!searchExpanded)` branch.
6385
*/
6486
collapse() {
65-
this.searchField.nativeElement.blur();
87+
this.searchField?.nativeElement?.blur();
6688
this.searchExpanded = false;
6789
this.isExpanded = 'collapsed';
6890
}
6991

7092
/**
71-
* Focuses on input search bar so search can be edited
93+
* Focuses the input search bar so it can be edited.
7294
*/
7395
editSearch(): void {
74-
this.searchField.nativeElement.focus();
96+
this.searchField?.nativeElement?.focus();
7597
}
7698

7799
/**
78-
* Submits the search (on enter or on search icon click)
100+
* Submits the search. Triggered both by Enter on the input (native form submit) and
101+
* by clicking the magnifying-glass icon (`<button type="submit">` inside the form).
79102
* @param data Data for the searchForm, containing the search query
80103
*/
81104
onSubmit(data: any) {
82-
this.collapse();
83105
const queryParams = Object.assign({}, data);
84106
const linkToNavigateTo = [this.searchService.getSearchLink().replace('/', '')];
85107
this.searchForm.reset();
108+
this.collapse();
86109

87110
this.router.navigate(linkToNavigateTo, {
88111
queryParams: queryParams,

0 commit comments

Comments
 (0)