Skip to content

Clean up Header component #4013

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

Merged
merged 5 commits into from
Oct 14, 2021
Merged

Clean up Header component #4013

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
0422966
Header: Use named grid areas
Turbo87 Oct 14, 2021
7bd17ce
Header: Use `aria-label` instead of a hidden `<label>` element
Turbo87 Oct 14, 2021
b19b0fb
Header: Add `role=search` to search forms
Turbo87 Oct 14, 2021
761dea4
Header: Merge search forms into a single form
Turbo87 Oct 14, 2021
9e9134a
Header: Adjust margins and paddings
Turbo87 Oct 14, 2021
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
2 changes: 0 additions & 2 deletions .template-lintrc.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,4 @@

module.exports = {
extends: ['octane', 'a11y'],

pending: [{ moduleId: 'app/components/header', only: ['no-duplicate-landmark-elements'] }],
};
40 changes: 22 additions & 18 deletions app/components/header.hbs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,12 @@
<h1>crates.io</h1>
</LinkTo>

<form local-class="desktop-search-form" action='/search' {{on "submit" (prevent-default this.search)}} data-test-search-form>
<form local-class="search-form" action='/search' role="search" {{on "submit" (prevent-default this.search)}} data-test-search-form>
{{! template-lint-disable require-input-label}}
{{! disabled due to https://github.com/ember-template-lint/ember-template-lint/issues/2141 }}
<input
type="text"
local-class="search"
local-class="search-input-lg"
name="q"
id="cargo-desktop-search"
placeholder="Click or press 'S' to search..."
Expand All @@ -19,9 +21,26 @@
autofocus="autofocus"
spellcheck="false"
required
aria-label="Search"
data-test-search-input
>
<label for="cargo-desktop-search" local-class="search-label">Search</label>

{{! Second input fields for narrow screens because CSS does not allow us to change the placeholder }}
<input
type="text"
local-class="search-input-sm"
name="q"
placeholder="Search"
value={{this.header.searchValue}}
oninput={{this.updateSearchValue}}
autocorrect="off"
required
aria-label="Search"
>

{{! Hidden submit button to enable enter-to-submit behavior for form with multiple inputs }}
<button type="submit" local-class="submit-button">Submit</button>

{{on-key 's' (focus '#cargo-desktop-search')}}
{{on-key 'S' (focus '#cargo-desktop-search')}}
{{on-key 'shift+s' (focus '#cargo-desktop-search')}}
Expand Down Expand Up @@ -142,20 +161,5 @@
</dd.Menu>
</Dropdown>
</div>

<form local-class='mobile-search' action='/search' {{on "submit" (prevent-default this.search)}}>
<input
type="text"
local-class="search"
name="q"
id="cargo-mobile-search"
placeholder="Search"
value={{this.header.searchValue}}
oninput={{this.updateSearchValue}}
autocorrect="off"
required
>
<label for="cargo-mobile-search" local-class="search-label">Search</label>
</form>
</div>
</header>
69 changes: 40 additions & 29 deletions app/components/header.module.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,18 +8,34 @@
composes: width-limit from '../styles/application.module.css';

display: grid;
grid-template-columns: auto 1fr auto;
grid-template:
"logo search nav" auto /
auto 1fr auto;
align-items: center;
padding: 10px;
padding: 10px 20px;
color: white;

a {
color: white; text-decoration: none;
&:hover { color: white; }
}

@media only screen and (max-width: 900px) {
grid-template:
"logo search menu" auto /
auto 1fr auto;
}

@media only screen and (max-width: 820px) {
grid-template:
"logo menu" auto
"search search" auto /
auto 1fr;
}
}

.index-link {
grid-area: logo;
display: flex;
align-items: center;

Expand All @@ -31,25 +47,21 @@
.logo {
width: 60px;
height: auto;
/* negative margin to account for blank space in the image */
margin-left: -10px;
margin-right: 10px;
}

.desktop-search-form {
.search-form {
grid-area: search;
display: flex;
flex-grow: 1;

@media only screen and (max-width: 820px) {
display: none;
}
}

input.search {
.search-input {
font-size: 90%;
border: none;
color: black;
width: 100%;
margin-left: 16px;
margin-right: 16px;
padding: 5px 5px 5px 25px;
background-color: white;
background-image: url('/assets/search.png');
Expand All @@ -66,28 +78,28 @@ input.search {
}
}

.mobile-search {
display: none;
grid-row: 2;
grid-column: span 3;
margin: 0 10px 10px;
.search-input-lg {
composes: search-input;
margin-left: 16px;
margin-right: 16px;

input.search {
margin: 0;
@media only screen and (max-width: 820px) {
display: none;
}
}

.search-input-sm {
composes: search-input;
display: none;
margin: 10px 0;

@media only screen and (max-width: 820px) {
display: block;
display: unset;
}
}

.search-label {
position: absolute;
left: -10000px;
top: auto;
width: 1px;
height: 1px;
overflow: hidden;
.submit-button {
display: none;
}

.sep {
Expand All @@ -101,20 +113,19 @@ input.search {
}

.nav {
grid-area: nav;
display: flex;
align-items: center;
text-align: right;
margin-right: 5px;

@media only screen and (max-width: 900px) {
display: none;
}
}

.menu {
grid-area: menu;
text-align: right;
margin-right: 5px;
flex-grow: 2;
display: none;

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