Skip to content

Commit e34e3e2

Browse files
committed
Auto merge of #4013 - Turbo87:header-cleanup, r=locks
Clean up `Header` component This PR cleans up our `<Header />` component. The primary goal was to fix the last remaining template-lint warning about having two search forms on the page. Instead of having two, we now have only one, but with two input fields inside of it, to allow us to use different placeholders, depending on the screen size. Visually, the header should still look essentially the same as before, except for a few small padding/margin adjustments.
2 parents 14183de + 9e9134a commit e34e3e2

File tree

3 files changed

+62
-49
lines changed

3 files changed

+62
-49
lines changed

.template-lintrc.js

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,4 @@
22

33
module.exports = {
44
extends: ['octane', 'a11y'],
5-
6-
pending: [{ moduleId: 'app/components/header', only: ['no-duplicate-landmark-elements'] }],
75
};

app/components/header.hbs

Lines changed: 22 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,12 @@
55
<h1>crates.io</h1>
66
</LinkTo>
77

8-
<form local-class="desktop-search-form" action='/search' {{on "submit" (prevent-default this.search)}} data-test-search-form>
8+
<form local-class="search-form" action='/search' role="search" {{on "submit" (prevent-default this.search)}} data-test-search-form>
9+
{{! template-lint-disable require-input-label}}
10+
{{! disabled due to https://github.com/ember-template-lint/ember-template-lint/issues/2141 }}
911
<input
1012
type="text"
11-
local-class="search"
13+
local-class="search-input-lg"
1214
name="q"
1315
id="cargo-desktop-search"
1416
placeholder="Click or press 'S' to search..."
@@ -19,9 +21,26 @@
1921
autofocus="autofocus"
2022
spellcheck="false"
2123
required
24+
aria-label="Search"
2225
data-test-search-input
2326
>
24-
<label for="cargo-desktop-search" local-class="search-label">Search</label>
27+
28+
{{! Second input fields for narrow screens because CSS does not allow us to change the placeholder }}
29+
<input
30+
type="text"
31+
local-class="search-input-sm"
32+
name="q"
33+
placeholder="Search"
34+
value={{this.header.searchValue}}
35+
oninput={{this.updateSearchValue}}
36+
autocorrect="off"
37+
required
38+
aria-label="Search"
39+
>
40+
41+
{{! Hidden submit button to enable enter-to-submit behavior for form with multiple inputs }}
42+
<button type="submit" local-class="submit-button">Submit</button>
43+
2544
{{on-key 's' (focus '#cargo-desktop-search')}}
2645
{{on-key 'S' (focus '#cargo-desktop-search')}}
2746
{{on-key 'shift+s' (focus '#cargo-desktop-search')}}
@@ -142,20 +161,5 @@
142161
</dd.Menu>
143162
</Dropdown>
144163
</div>
145-
146-
<form local-class='mobile-search' action='/search' {{on "submit" (prevent-default this.search)}}>
147-
<input
148-
type="text"
149-
local-class="search"
150-
name="q"
151-
id="cargo-mobile-search"
152-
placeholder="Search"
153-
value={{this.header.searchValue}}
154-
oninput={{this.updateSearchValue}}
155-
autocorrect="off"
156-
required
157-
>
158-
<label for="cargo-mobile-search" local-class="search-label">Search</label>
159-
</form>
160164
</div>
161165
</header>

app/components/header.module.css

Lines changed: 40 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -8,18 +8,34 @@
88
composes: width-limit from '../styles/application.module.css';
99

1010
display: grid;
11-
grid-template-columns: auto 1fr auto;
11+
grid-template:
12+
"logo search nav" auto /
13+
auto 1fr auto;
1214
align-items: center;
13-
padding: 10px;
15+
padding: 10px 20px;
1416
color: white;
1517

1618
a {
1719
color: white; text-decoration: none;
1820
&:hover { color: white; }
1921
}
22+
23+
@media only screen and (max-width: 900px) {
24+
grid-template:
25+
"logo search menu" auto /
26+
auto 1fr auto;
27+
}
28+
29+
@media only screen and (max-width: 820px) {
30+
grid-template:
31+
"logo menu" auto
32+
"search search" auto /
33+
auto 1fr;
34+
}
2035
}
2136

2237
.index-link {
38+
grid-area: logo;
2339
display: flex;
2440
align-items: center;
2541

@@ -31,25 +47,21 @@
3147
.logo {
3248
width: 60px;
3349
height: auto;
50+
/* negative margin to account for blank space in the image */
51+
margin-left: -10px;
3452
margin-right: 10px;
3553
}
3654

37-
.desktop-search-form {
55+
.search-form {
56+
grid-area: search;
3857
display: flex;
39-
flex-grow: 1;
40-
41-
@media only screen and (max-width: 820px) {
42-
display: none;
43-
}
4458
}
4559

46-
input.search {
60+
.search-input {
4761
font-size: 90%;
4862
border: none;
4963
color: black;
5064
width: 100%;
51-
margin-left: 16px;
52-
margin-right: 16px;
5365
padding: 5px 5px 5px 25px;
5466
background-color: white;
5567
background-image: url('/assets/search.png');
@@ -66,28 +78,28 @@ input.search {
6678
}
6779
}
6880

69-
.mobile-search {
70-
display: none;
71-
grid-row: 2;
72-
grid-column: span 3;
73-
margin: 0 10px 10px;
81+
.search-input-lg {
82+
composes: search-input;
83+
margin-left: 16px;
84+
margin-right: 16px;
7485

75-
input.search {
76-
margin: 0;
86+
@media only screen and (max-width: 820px) {
87+
display: none;
7788
}
89+
}
90+
91+
.search-input-sm {
92+
composes: search-input;
93+
display: none;
94+
margin: 10px 0;
7895

7996
@media only screen and (max-width: 820px) {
80-
display: block;
97+
display: unset;
8198
}
8299
}
83100

84-
.search-label {
85-
position: absolute;
86-
left: -10000px;
87-
top: auto;
88-
width: 1px;
89-
height: 1px;
90-
overflow: hidden;
101+
.submit-button {
102+
display: none;
91103
}
92104

93105
.sep {
@@ -101,20 +113,19 @@ input.search {
101113
}
102114

103115
.nav {
116+
grid-area: nav;
104117
display: flex;
105118
align-items: center;
106119
text-align: right;
107-
margin-right: 5px;
108120

109121
@media only screen and (max-width: 900px) {
110122
display: none;
111123
}
112124
}
113125

114126
.menu {
127+
grid-area: menu;
115128
text-align: right;
116-
margin-right: 5px;
117-
flex-grow: 2;
118129
display: none;
119130

120131
@media only screen and (max-width: 900px) {

0 commit comments

Comments
 (0)