Skip to content

Accessibility fixes (and tests) #1227

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 24 commits into from
Jan 17, 2018
Merged

Accessibility fixes (and tests) #1227

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
0929810
fix(templates/application): Add labels to search input fields
sorin-davidoi Jan 9, 2018
c00715f
fix(templates/application): Logo should not be focusable
sorin-davidoi Jan 9, 2018
dd877cc
fix(styles/home/button): Use hover style for focus
sorin-davidoi Jan 9, 2018
d3ff8cd
fix(templates/application): Use semantics HTML tags (nav, main, footer)
sorin-davidoi Jan 9, 2018
bd4c191
fix(templates/index): Use section for each crate list
sorin-davidoi Jan 9, 2018
89c8ede
fix(templates/components/crate-list): Use ol instead of ul
sorin-davidoi Jan 9, 2018
3e94117
fix(templates/application): Remove tabindex="1" from search fields
sorin-davidoi Jan 9, 2018
f0c8377
fix(templates/application): Add alt text for the "For me on GitHub" link
sorin-davidoi Jan 9, 2018
72407de
fix(styles/app/button): Do not set outline: 0
sorin-davidoi Jan 9, 2018
522eb3e
fix(styles/me): Set focus style for load more button
sorin-davidoi Jan 9, 2018
1aa4b29
fix(templates/application): Move fork button to the end of the document
sorin-davidoi Jan 9, 2018
ea4fb9c
fix(componentns/user-avatar): Add alt text (name, username)
sorin-davidoi Jan 9, 2018
961ba48
fix(crate/version): Nest li directly inside ul
sorin-davidoi Jan 9, 2018
7c58178
fix(templates/crate/version): Add title to copy-to-clipboard button
sorin-davidoi Jan 9, 2018
c27533d
feat(templates/crate/version): Use section for readme and authorship
sorin-davidoi Jan 9, 2018
4a36d50
fix(templates/crate/owners): Add label to username input field
sorin-davidoi Jan 9, 2018
274c841
chore(npm/dev-dependencies): Install ember-a11y-testing
sorin-davidoi Jan 10, 2018
ed4333a
test(acceptance): Create accessibility tests
sorin-davidoi Jan 10, 2018
1ff0a23
feat(config/environment): Disable accessibility audit in during devel…
sorin-davidoi Jan 10, 2018
fc5a680
fix(templates/index): Use aria-busy and show loader using CSS
sorin-davidoi Jan 10, 2018
2cd5ac1
fix(templates/crates): Make pagination accessible
sorin-davidoi Jan 10, 2018
8d2157a
fix(templates/crate/version): Change aria-label to Crate metadata
sorin-davidoi Jan 14, 2018
e7a8209
fix(styles/home): Update custom home crate style from div to section
sorin-davidoi Jan 14, 2018
0682d78
fix(themes/application): Invalid markup
sorin-davidoi Jan 17, 2018
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: 5 additions & 1 deletion app/components/user-avatar.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { computed } from '@ember/object';
export default Component.extend({
size: 'small',
user: null,
attributeBindings: ['src', 'width', 'height'],
attributeBindings: ['src', 'width', 'height', 'alt'],
tagName: 'img',

width: computed('size', function() {
Expand All @@ -20,6 +20,10 @@ export default Component.extend({

height: readOnly('width'),

alt: computed('user', function() {
return `${this.get('user.name')} (${this.get('user.login')})`;
}),

src: computed('size', 'user', function() {
return `${this.get('user.avatar')}&s=${this.get('width') * 2}`;
})
Expand Down
11 changes: 9 additions & 2 deletions app/styles/app.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,6 @@ body {

button {
background: none;
outline: 0;
border: 0;
padding: 10px 0;

Expand Down Expand Up @@ -130,7 +129,6 @@ body {
font-size: 90%;
border: none;
color: black;
outline: 0;
margin-left: 30px;
padding: 5px 5px 5px 25px;
background-color: white;
Expand All @@ -141,6 +139,15 @@ body {
@include border-radius(15px);
}

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

#header input.search {
width: 100%;
margin-left: 16px;
Expand Down
6 changes: 6 additions & 0 deletions app/styles/crate.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,12 @@
font-size: 90%;
margin-bottom: 20px;

ol {
list-style: none;
padding: 0;
}
ol, li { display: inline; }

a {
color: $main-color-light;
text-decoration: none;
Expand Down
16 changes: 12 additions & 4 deletions app/styles/home.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
margin-right: 10px;
}

&:hover { @include vertical-gradient($start_dark, $end_dark); outline: 0; }
&:hover, &:focus { @include vertical-gradient($start_dark, $end_dark); outline: 0; }
&.active { @include vertical-gradient($start_dark, $end_dark); outline: 0; }
&[disabled] {
@include vertical-gradient($start_light, $end_light);
Expand Down Expand Up @@ -111,9 +111,17 @@ button.small {
line-height: 20px;
}

> div { margin: 0 15px; }
> section { margin: 0 15px; }
Copy link
Member

Choose a reason for hiding this comment

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

I think the div on line 146 of this file needs to be updated to a section as well, otherwise the columns on the home page take up a variable amount of width when they should be taking up ~1/3 of the width, which starts happening at this commit.


ul { list-style: none; padding: 0; }
> section[aria-busy="true"] > h2::after {
content: '';
background-image: url('/assets/ajax-loader.gif');
display: inline-block;
height: 16px;
width: 16px;
}

ul, ol { list-style: none; padding: 0; }
li { margin: 8px 0; }

li a {
Expand Down Expand Up @@ -143,7 +151,7 @@ button.small {
@include flex-wrap(wrap);
@include justify-content(left);

> div {
> section {
margin: 0;
padding: 0 15px;
width: 33.33%;
Expand Down
2 changes: 1 addition & 1 deletion app/styles/me.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@
margin-bottom: 20px;
background-color: $bg;
color: white;
&:hover { background-color: darken($bg, 10%); }
&:hover, &:focus { background-color: darken($bg, 10%); }
}
}

Expand Down
28 changes: 14 additions & 14 deletions app/templates/application.hbs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,8 @@
{{title "Cargo: packages for Rust" separator=' - ' prepend=true}}
{{google-jsapi}}

<a href='https://github.com/rust-lang/crates.io' class='fork-me'>
<img src='/assets/forkme.png'/>
</a>

<div id="header">
{{#link-to "index"}}
<nav id="header">
{{#link-to "index" tabindex="-1"}}
<img src="/assets/Cargo-Logo-Small.png" id="logo"
height=100 width=100 alt="Cargo Logo"/>
{{/link-to}}
Expand All @@ -23,9 +19,9 @@
value={{searchQuery}}
oninput={{action (mut searchQuery) value="target.value"}}
autofocus="autofocus"
tabindex="1"
required
data-test-search-input>
<label for="cargo-desktop-search">Search</label>
</form>

<div class='nav'>
Expand Down Expand Up @@ -100,25 +96,25 @@

<div class='links'>
</div>
</div>
</nav>

<form id='mobile-search' class='search' action='/search' {{ action "search" on="submit" }} >
<input type="text" class="search" name="q"
<input type="text" class="search" name="q" id="cargo-mobile-search"
placeholder="Search"
value={{searchQuery}}
oninput={{action (mut searchQuery) value="target.value"}}
autocorrect="off"
tabindex="1"
required>
<label for="cargo-mobile-search">Search</label>
</form>

<div id="main" class='inner-content'>
<main id="main" class='inner-content'>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is not ideal since each page should define it's own main - but I'm not familiar enough with the pages and don't have time to go through them all, so I think this is good enough for now.

{{flash-message}}

{{outlet}}
</div>
</main>

<div class='after-main-links'>
<footer class='after-main-links'>
{{#link-to 'install'}}Install{{/link-to}}
<span class="sep">|</span>
<a href='http://doc.crates.io'>Getting Started</a>
Expand All @@ -128,4 +124,8 @@
<a href='mailto:[email protected]'>Send us an email</a>
<span class="sep">|</span>
{{#link-to 'policies'}}Policies{{/link-to}}
</div>
</footer>

<a href='https://github.com/rust-lang/crates.io' class='fork-me'>
<img src='/assets/forkme.png' alt="Fork me on GitHub" />
</a>
4 changes: 2 additions & 2 deletions app/templates/components/crate-list.hbs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<ul>
<ol>
Copy link
Member

Choose a reason for hiding this comment

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

Should keyword-list and category-list be changed to ols too? They're both ordered by popularity, and those components are only used on the home page.

What about crate-downloads-list? This is used on the currently-logged-in user's dashboard and are ordered by recent downloads.

Not sure how far we should go with this in this PR-- there are lists of crates that are just divs rather than uls right now that should also perhaps be ols, like every list of crates (which should always have SOME order) or the list of Latest Updates of crates you're following on your dashboard

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, probably those need to be updated as well. Would rather do it in a follow-up PR if that's OK.

Copy link
Member

Choose a reason for hiding this comment

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

Yup, that's totally fine!

{{#each crates as |crate index|}}
<li>
{{#link-to 'crate' crate.id class='name' data-test-crate-link=index}}
Expand All @@ -9,4 +9,4 @@
{{/link-to}}
</li>
{{/each}}
</ul>
</ol>
14 changes: 6 additions & 8 deletions app/templates/components/link-to-dep.hbs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
<li>
{{#link-to 'crate' dep.crate_id}}
{{ dep.crate_id }} {{ format-req dep.req }}
{{/link-to}}
{{#if dep.optional}}
<span class='optional'>optional</span>
{{/if}}
</li>
{{#link-to 'crate' dep.crate_id}}
{{ dep.crate_id }} {{ format-req dep.req }}
{{/link-to}}
{{#if dep.optional}}
<span class='optional'>optional</span>
{{/if}}
9 changes: 4 additions & 5 deletions app/templates/crate/owners.hbs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,11 @@
<h2>Add Owner</h2>

<div class="row">
<div class="label">
<dt>Username</dt>
</div>

<form class="email-form" {{action 'addOwner' on='submit'}}>
{{input type='text' value=username placeholder='Username' class='form-control space-right' name='username'}}
<label class="label" for='new-owner-username'>
Username
</label>
{{input type='text' id='new-owner-username' value=username placeholder='Username' class='form-control space-right' name='username'}}

{{#if error}}
<div class='error'>
Expand Down
12 changes: 6 additions & 6 deletions app/templates/crate/version.hbs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@
<code id="crate-toml">{{ crate.name }} = "{{ currentVersion.num }}"</code>

{{#if (is-clipboard-supported)}}
{{#copy-button clipboardTarget="#crate-toml" success=(action 'copySuccess') error=(action 'copyError')}}
{{#copy-button clipboardTarget="#crate-toml" success=(action 'copySuccess') error=(action 'copyError') title="Copy to clipboard"}}
{{svg-jar "copy" alt="Copy to clipboard"}}
Copy link
Member

@carols10cents carols10cents Jan 14, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

The title on the button isn't repetitive with the alt text on the svg inside the button? I really don't know how screenreaders will read this, just wanted to point it out :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

aXe flagged this as an error, and Orca (the screen reader I test with) didn't read the text from the svg.

{{/copy-button}}
{{/if}}
Expand All @@ -92,9 +92,9 @@
</span>
</div>
{{#if crate.readme}}
<div class="crate-readme">
<section class="crate-readme" aria-label="Readme">
{{crate-readme rendered=crate.readme}}
</div>
</section>
{{else}}
{{#if crate.description}}
<div class='about'>
Expand All @@ -104,7 +104,7 @@
{{/if}}
{{/if}}
</div>
<div class='authorship'>
<section class='authorship' aria-label="Crate metadata">
<div class='top'>
<div>
<div class='last-update'><span class='small'>Last Updated</span></div>
Expand Down Expand Up @@ -225,7 +225,7 @@
<h3>Dependencies</h3>
<ul>
{{#each currentDependencies as |dep|}}
{{link-to-dep dep=dep}}
{{link-to-dep tagName="li" dep=dep}}
{{else}}
<li>None</li>
{{/each}}
Expand All @@ -243,7 +243,7 @@
</div>
{{/if}}
</div>
</div>
</section>
</div>

<div id='crate-downloads'>
Expand Down
18 changes: 11 additions & 7 deletions app/templates/crates.hbs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,16 +71,20 @@
{{/each}}
</div>

<div class='pagination'>
<nav class='pagination' aria-label="Pagination navigation">
{{#link-to (query-params page=prevPage) class="prev" rel="prev" title="previous page" data-test-pagination-prev=true}}
{{svg-jar "left-pag"}}
{{/link-to}}
{{#each pages as |page|}}
{{#link-to (query-params page=page)}}
{{ page }}
{{/link-to}}
{{/each}}
<ol>
{{#each pages as |page|}}
<li>
{{#link-to (query-params page=page) title=(concat "Go to page " page)}}
{{ page }}
{{/link-to}}
</li>
{{/each}}
</ol>
{{#link-to (query-params page=nextPage) class="next" rel="next" title="next page" data-test-pagination-next=true}}
{{svg-jar "right-pag"}}
{{/link-to}}
</div>
</nav>
36 changes: 18 additions & 18 deletions app/templates/index.hbs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,28 +36,28 @@
</div>

<div id='home-crates' class='crate-lists'>
<div id='new-crates' data-test-new-crates>
<h2>New Crates {{#if dataTask.isRunning}}<img src="/assets/ajax-loader.gif">{{/if}}</h2>
<section id='new-crates' data-test-new-crates aria-busy="{{dataTask.isRunning}}">
<h2>New Crates</h2>
{{crate-list crates=model.new_crates}}
</div>
<div id='most-downloaded' data-test-most-downloaded>
<h2>Most Downloaded {{#if dataTask.isRunning}}<img src="/assets/ajax-loader.gif">{{/if}}</h2>
</section>
<section id='most-downloaded' data-test-most-downloaded aria-busy="{{dataTask.isRunning}}">
<h2>Most Downloaded</h2>
{{crate-list crates=model.most_downloaded}}
</div>
<div id='just-updated' data-test-just-updated>
<h2>Just Updated {{#if dataTask.isRunning}}<img src="/assets/ajax-loader.gif">{{/if}}</h2>
</section>
<section id='just-updated' data-test-just-updated aria-busy="{{dataTask.isRunning}}">
<h2>Just Updated</h2>
{{crate-list crates=model.just_updated}}
</div>
<div id='most-recently-downloaded' data-test-most-recently-downloaded>
<h2>Most Recent Downloads {{#if dataTask.isRunning}}<img src="/assets/ajax-loader.gif">{{/if}}</h2>
</section>
<section id='most-recently-downloaded' data-test-most-recently-downloaded aria-busy="{{dataTask.isRunning}}">
<h2>Most Recent Downloads</h2>
{{crate-list crates=model.most_recently_downloaded}}
</div>
<div id='keywords' data-test-keywords>
<h2>Popular Keywords {{#link-to 'keywords'}}(see all){{/link-to}} {{#if dataTask.isRunning}}<img src="/assets/ajax-loader.gif">{{/if}}</h2>
</section>
<section id='keywords' data-test-keywords aria-busy="{{dataTask.isRunning}}">
<h2>Popular Keywords {{#link-to 'keywords'}}(see all){{/link-to}}</h2>
{{keyword-list keywords=model.popular_keywords}}
</div>
<div id='categories' data-test-categories>
<h2>Popular Categories {{#link-to 'categories'}}(see all){{/link-to}} {{#if dataTask.isRunning}}<img src="/assets/ajax-loader.gif">{{/if}}</h2>
</section>
<section id='categories' data-test-categories aria-busy="{{dataTask.isRunning}}">
<h2>Popular Categories {{#link-to 'categories'}}(see all){{/link-to}}</h2>
{{category-list categories=model.popular_categories}}
</div>
</section>
</div>
5 changes: 5 additions & 0 deletions config/environment.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,11 @@ module.exports = function(environment) {
// ENV.APP.LOG_TRANSITIONS = true;
// ENV.APP.LOG_TRANSITIONS_INTERNAL = true;
// ENV.APP.LOG_VIEW_LOOKUPS = true;
ENV['ember-a11y-testing'] = {
componentOptions: {
turnAuditOff: true,
}
};
}

if (environment === 'test') {
Expand Down
Loading