-
-
Notifications
You must be signed in to change notification settings - Fork 398
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.
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
Accessibility improvements #1501
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -82,6 +82,11 @@ body { | |
#search { display: none; } | ||
} | ||
|
||
@media (max-width: 320px) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We had a requirement to support devices with < 320 px width (like smart phone). I'm not a CSS expert, but the way this is coded in style.css for 920px min-width interferes with 320px unless I copy over the same properties. I effectively am only adding break-word here. I am probably not the best person to untangle / cascade this properly. It removes all horizontal scrolling except for tables and code snippets which is acceptable for the requirement. |
||
body { height: 100%; overflow: hidden; overflow-wrap: break-word; } | ||
#main { height: 100%; overflow: auto; } | ||
} | ||
|
||
#main img { max-width: 100%; } | ||
h1 { font-size: 25px; margin: 1em 0 0.5em; padding-top: 4px; border-top: 1px dotted #d5d5d5; } | ||
h1.noborder { border-top: 0px; margin-top: 0; padding-top: 4px; } | ||
|
@@ -106,6 +111,7 @@ h2 small a { | |
position: relative; | ||
padding: 2px 7px; | ||
} | ||
a { font-weight: 550; } | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bold links are 700, normal is 400. This is a weight in between. The requirement was to make links more obvious for people with low vision. |
||
.clear { clear: both; } | ||
.inline { display: inline; } | ||
.inline p:first-child { display: inline; } | ||
|
@@ -382,6 +388,16 @@ ul.fullTree li:last-child { padding-bottom: 0; } | |
text-align: center; | ||
} | ||
|
||
.visually-hidden { | ||
position: absolute; | ||
top: auto; | ||
overflow: hidden; | ||
clip: rect(1px, 1px, 1px, 1px); | ||
width: 1px; | ||
height: 1px; | ||
white-space: nowrap; | ||
} | ||
|
||
#menu { font-size: 1.3em; color: #bbb; } | ||
#menu .title, #menu a { font-size: 0.7em; } | ||
#menu .title a { font-size: 1em; } | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,5 @@ | ||
<!DOCTYPE html> | ||
<html> | ||
<html <%= "lang=\"#{html_lang}\"" unless html_lang.nil? %>> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We hardcoded lang="en" in our docs, but I think it makes sense to have this as a setup method. |
||
<head> | ||
<meta name="viewport" content="width=device-width, initial-scale=1.0"> | ||
<meta charset="<%= charset %>" /> | ||
|
@@ -26,7 +26,10 @@ | |
<% end %> | ||
</div> | ||
|
||
<div id="search">Search: <input type="text" /></div> | ||
<div id="search"> | ||
<label for="search-class">Search:</label> | ||
<input id="search-class" type="text" /> | ||
</div> | ||
Comment on lines
+29
to
+32
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is just adding a label for search for screen readers to know what it's for. |
||
</div> | ||
|
||
<ul id="full_list" class="<%= @list_class || @list_type %>"> | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -62,8 +62,25 @@ function enableToggles() { | |
evt.stopPropagation(); | ||
evt.preventDefault(); | ||
$(this).parent().parent().toggleClass('collapsed'); | ||
$(this).attr('aria-expanded', function (i, attr) { | ||
return attr == 'true' ? 'false' : 'true' | ||
}); | ||
highlight(); | ||
}); | ||
|
||
// navigation of nested classes using keyboard | ||
$('#full_list a.toggle').on('keypress',function(evt) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Part of this change allows you to tab navigate to the arrow/carrot in the full list menu, then you can press enter to expand the section without visiting a new page. |
||
// enter key is pressed | ||
if (evt.which == 13) { | ||
evt.stopPropagation(); | ||
evt.preventDefault(); | ||
$(this).parent().parent().toggleClass('collapsed'); | ||
$(this).attr('aria-expanded', function (i, attr) { | ||
return attr == 'true' ? 'false' : 'true' | ||
}); | ||
highlight(); | ||
} | ||
}); | ||
} | ||
|
||
function populateSearchCache() { | ||
|
@@ -91,7 +108,7 @@ function enableSearch() { | |
} | ||
}); | ||
|
||
$('#full_list').after("<div id='noresults' style='display:none'></div>"); | ||
$('#full_list').after("<div id='noresults' role='status' style='visually-hidden'></div>"); | ||
} | ||
|
||
function ignoredKeyPress(event) { | ||
|
@@ -119,7 +136,7 @@ function clearSearch() { | |
function performSearch(searchString) { | ||
clearSearchTimeout(); | ||
$('#full_list, #content').addClass('insearch'); | ||
$('#noresults').text('').hide(); | ||
$('#noresults').text('').addClass('visually-hidden'); | ||
partialSearch(searchString, 0); | ||
} | ||
|
||
|
@@ -154,11 +171,14 @@ function partialSearch(searchString, offset) { | |
function searchDone() { | ||
searchTimeout = null; | ||
highlight(); | ||
if ($('#full_list li:visible').size() === 0) { | ||
$('#noresults').text('No results were found.').hide().fadeIn(); | ||
var found = $('#full_list li:visible').size(); | ||
if (found === 0) { | ||
$('#noresults').text('No results were found.'); | ||
} else { | ||
$('#noresults').text('').hide(); | ||
// This is read out to screen readers | ||
$('#noresults').text('There are ' + found + ' results.'); | ||
} | ||
$('#noresults').removeClass('visually-hidden'); | ||
$('#content').removeClass('insearch'); | ||
} | ||
|
||
|
@@ -188,6 +208,12 @@ function expandTo(path) { | |
$target.addClass('clicked'); | ||
$target.removeClass('collapsed'); | ||
$target.parentsUntil('#full_list', 'li').removeClass('collapsed'); | ||
|
||
$target.find('a.toggle').attr('aria-expanded', 'true') | ||
$target.parentsUntil('#full_list', 'li').each(function(i, el) { | ||
$(el).find('> div > a.toggle').attr('aria-expanded', 'true'); | ||
}); | ||
|
||
if($target[0]) { | ||
window.scrollTo(window.scrollX, $target.offset().top - 250); | ||
highlight(); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -104,6 +104,12 @@ def javascripts_full_list | |
%w(js/jquery.js js/full_list.js) | ||
end | ||
|
||
# Sets the HTML language lang="value" where value is the value returned from | ||
# this method. Defaults to nil which does not set the lang attribute. | ||
def html_lang | ||
nil | ||
end | ||
|
||
def menu_lists | ||
Object.new.extend(T('layout')).menu_lists | ||
end | ||
|
@@ -225,15 +231,17 @@ def class_list(root = Registry.root, tree = TreeContext.new) | |
has_children = run_verifier(child.children).any? {|o| o.is_a?(CodeObjects::NamespaceObject) } | ||
out << "<li id='object_#{child.path}' class='#{tree.classes.join(' ')}'>" | ||
out << "<div class='item' style='padding-left:#{tree.indent}'>" | ||
out << "<a class='toggle'></a> " if has_children | ||
accessible_props = "aria-label='#{name} child nodes' aria-expanded='false' aria-controls='object_#{child.path}'" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Setting standard aria properties. The JS code will flip these. |
||
out << "<a tabindex='0' class='toggle' role='button' #{accessible_props}></a> " if has_children | ||
out << linkify(child, name) | ||
out << " < #{child.superclass.name}" if child.is_a?(CodeObjects::ClassObject) && child.superclass | ||
out << "<small class='search_info'>" | ||
out << child.namespace.title | ||
out << "</small>" | ||
out << "</div>" | ||
tree.nest do | ||
out << "<ul>#{class_list(child, tree)}</ul>" if has_children | ||
labeled_by = "aria-labelledby='object_#{child.path}'" | ||
out << "<div #{labeled_by}><ul>#{class_list(child, tree)}</ul></div>" if has_children | ||
end | ||
out << "</li>" | ||
end | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,9 +1,9 @@ | ||
<% if object.has_tag?(:example) %> | ||
<div class="examples"> | ||
<p class="tag_title">Examples:</p> | ||
<h4 class="tag_title">Examples:</h4> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. One requirement we had was to not have paragraph sections for examples. We set these to h4 and h5 headers so I think this is reasonable as they are supposed to be sections. |
||
<% object.tags(:example).each do |tag| %> | ||
<% unless tag.name.empty? %> | ||
<p class="example_title"><%= htmlify_line(tag.name) %></p> | ||
<h5 class="example_title"><%= htmlify_line(tag.name) %></h5> | ||
<% end %> | ||
<pre class="example code"><code><%= html_syntax_highlight(tag.text) %></code></pre> | ||
<% end %> | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Opacity was removed and color changed from 888 to 666 to allow for at least a 4.5 contrast ratio (accessibility requirement). It looks roughly the same, just a little darker.