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.

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

Screen Reader Accessibility #5

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
+72 −50
Open

Screen Reader Accessibility #5

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b3892c2
Screen Reader Accessibility
jpdevries Nov 12, 2016
c34522d
removing #js-shopping-list-add-item from CSS
jpdevries Feb 6, 2017
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
95 changes: 47 additions & 48 deletions index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,58 +13,57 @@
<div class="container">
<h1>Shopping List</h1>

<form id="js-shopping-list-form">
<label for="shopping-list-entry">Add an item</label>
<form id="js-shopping-list-add-item">
Copy link
Contributor

Choose a reason for hiding this comment

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

The js- prefix is reserved for ids and classes that are to be targeted by JS layer. Shouldn't be used for CSS. The one that's in master right now should actually be gone altogether, so if you need an id here for CSS, can create new one.

Copy link
Author

Choose a reason for hiding this comment

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

@benjaminEwhite just loosened the CSS selector to not reference the js id

<h2><label for="shopping-list-entry">Add an item</label></h2>
<input type="text" name="shopping-list-entry" id="shopping-list-entry" placeholder="e.g., broccoli">
<button type="submit">Add item</button>
</form>

<ul class="shopping-list">
<li>
<span class="shopping-item">apples</span>
<div class="shopping-item-controls">
<button class="shopping-item-toggle">
<span class="button-label">check</span>
</button>
<button class="shopping-item-delete">
<span class="button-label">delete</span>
</button>
</div>
</li>
<li>
<span class="shopping-item">oranges</span>
<div class="shopping-item-controls">
<button class="shopping-item-toggle">
<span class="button-label">check</span>
</button>
<button class="shopping-item-delete">
<span class="button-label">delete</span>
</button>
</div>
</li>
<li>
<span class="shopping-item shopping-item__checked">milk</span>
<div class="shopping-item-controls">
<button class="shopping-item-toggle">
<span class="button-label">check</span>
</button>
<button class="shopping-item-delete">
<span class="button-label">delete</span>
</button>
</div>
</li>
<li>
<span class="shopping-item">bread</span>
<div class="shopping-item-controls">
<button class="shopping-item-toggle">
<span class="button-label">check</span>
</button>
<button class="shopping-item-delete">
<span class="button-label">delete</span>
</button>
</div>
</li>
</ul>
<form id="js-shopping-list-form">
<ul class="shopping-list">
<li id="apples">
Copy link

@mxmason mxmason Feb 2, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

Would a data-* attribute not be a better way to name each list item?

I feel like it would be if we didn't need unique IDs later for the form elements. Given this need, it's a good idea to use underscores to indicate states, etc., but I wanted to bring this up. Thoughts?

Copy link
Author

@jpdevries jpdevries Feb 6, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

@dengeist I think the advantage of using an ID is they double as a hash jump to link to scroll to and :target that element. In other words, the id attribute isn't just for CSS

Copy link
Contributor

Choose a reason for hiding this comment

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

The underlying issue here is that we don't want to lock the student into a given way of managing the state for this. If we have state being stored in ids or in data attributes in the template, they're just going to use that approach when they create their app.

@jpdevries can you please explain the reasoning behind the ids?

Copy link

Choose a reason for hiding this comment

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

Good point about offering too many opinions, @benjaminEwhite, but isn't this also a good time to teach the data-*attribute? This is a great use-case for it.

Copy link
Author

Choose a reason for hiding this comment

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

@benjaminEwhite the id isn't for managing state. it's for accessible jump-to :target links Uses the :target pseudo selector so for really long lists you can add #walnuts to the end of the URL and scroll half way down the list to that item to find that item highlighted.

Uses the :target pseudo selector so for really long lists you can add #walnuts to the end of the URL and scroll half way down the list to that item to find that item highlighted.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh that's nifty.

<h3 class="shopping-item">
<input type="checkbox" id="apples__completed" name="apples__completed">
<label for="apples__completed">apples</label>
</h3>
<div class="shopping-item-controls">
<label for="apples__deleted">Delete<span class="visually-hidden"> apples</span></label>
<input type="checkbox" id="apples__deleted" name="apples__deleted">
</div>
</li>
<li id="oranges">
<h3 class="shopping-item">
<input type="checkbox" id="oranges__completed" name="oranges__completed">
<label for="oranges__completed">oranges</label>
</h3>
<div class="shopping-item-controls">
<label for="oranges__deleted">Delete<span class="visually-hidden"> oranges</span></label>
<input type="checkbox" id="oranges__deleted" name="oranges__deleted">
</div>
</li>
<li id="milk">
<h3 class="shopping-item">
<input type="checkbox" id="milk__completed" name="milk__completed" checked>
<label for="milk__completed">milk</label>
</h3>
<div class="shopping-item-controls">
<label for="milk__deleted">Delete<span class="visually-hidden"> milk</span></label>
<input type="checkbox" id="milk__deleted" name="milk__deleted">
</div>
</li>
<li id="bread">
<h3 class="shopping-item">
<input type="checkbox" id="bread__completed" name="bread__completed">
<label for="bread__completed">bread</label>
</h3>
<div class="shopping-item-controls">
<label for="bread__deleted">Delete<span class="visually-hidden"> bread</span></label>
<input type="checkbox" id="bread__deleted" name="bread__deleted">
</div>
</li>
</ul>
<button type="button" name="button">Trash Deleted Items</button>
</form>
</div>

</body>
Expand Down
27 changes: 25 additions & 2 deletions main.css
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,15 @@
.visually-hidden { /*https://developer.yahoo.com/blogs/ydn/clip-hidden-content-better-accessibility-53456.html*/
position: absolute !important;
clip: rect(1px 1px 1px 1px); /* IE6, IE7 */
clip: rect(1px, 1px, 1px, 1px);
padding:0 !important;
border:0 !important;
height: 1px !important;
width: 1px !important;
overflow: hidden;
}
body:hover .visually-hidden a, body:hover .visually-hidden input, body:hover .visually-hidden button { display: none !important; }

* {
box-sizing: border-box;
}
Expand All @@ -18,6 +30,13 @@ button:hover {
width: 250px;
}

#js-shopping-list-add-item h2 {
Copy link
Contributor

Choose a reason for hiding this comment

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

see note above about js- prefix.

display: inline-block;
margin: 0;
font-size: 1em;
font-weight: normal;
}

.container {
max-width: 600px;
margin: 0 auto;
Expand All @@ -34,14 +53,18 @@ button:hover {
padding: 20px;
}

.shopping-list > li:target {
background: #f9ec8f;
}

.shopping-item {
display: block;
color: grey;
font-style: italic;
font-size: 20px;
margin-bottom: 15px;
margin: 0 0 15px 0;
}

.shopping-item__checked {
input[type="checkbox"]:checked + label {
Copy link
Contributor

Choose a reason for hiding this comment

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

cool

text-decoration: line-through;
}