Skip to content

E
edx-platform
Commit 2967f18d by Chris
Options

Merge pull request #6739 from edx/clrux/lms-navigation-accessibility-update 

Accessibility fixes for UX-1572
parents 1b96a597 3acb5c53
Showing with 81 additions and 192 deletions
common/lib/xmodule/xmodule/css/sequence/display.scss
...@@ -32,7 +32,7 @@ $sequence--border-color: #C8C8C8; ...@@ -32,7 +32,7 @@ $sequence--border-color: #C8C8C8;
// ==================== // ====================
nav.sequence-nav { .sequence-nav {
// TODO (cpennington): This doesn't work anymore. XModules aren't able to // TODO (cpennington): This doesn't work anymore. XModules aren't able to
// import from external sources. // import from external sources.
@extend .topbar; @extend .topbar;
...@@ -231,93 +231,63 @@ nav.sequence-nav { ...@@ -231,93 +231,63 @@ nav.sequence-nav {
} }
} }
ul { body.touch-based-device & ol li a:hover p {
position: absolute; display: none;
top: 0; }
list-style: none !important; }
height: 100%;
right: 0;
top: 0;
width: 100%;
margin: 0;
border: none;
li {
position: absolute;
margin-bottom: 0;
height: 44px;
width: 70px;
border: 1px solid $gray-l3;
@include linear-gradient(top, #eee, #ddd);
box-shadow: 0 1px 0 rgba(255, 255, 255, .7) inset;
z-index: 1;
&.prev, &.next {
a {
background-position: center;
background-repeat: no-repeat;
display: block;
height: 100%;
width: 40px;
text-indent: -9999px;
overflow: hidden;
@include transition(all .2s $ease-in-out-quad 0s);
&:hover, &:focus {
opacity: 0.5;
}
&.disabled {
cursor: normal;
opacity: 0.4;
}
}
}
&.prev { .sequence-nav-button {
@include left(-10px); @extend %ui-depth3;
@include border-radius(35px, 0, 0, 35px); position: absolute;
display: block;
top: 0;
width: ($baseline*2);
height: 46px;
background-position: center;
background-repeat: no-repeat;
text-indent: -9999px;
overflow: hidden;
@include transition(all .2s $ease-in-out-quad 0s);
&.button-previous {
@include border-radius(35px, 0, 0, 35px);
left: 0;
background-position: center 15px;
a { // CASE: left to right layout
background-position: center 15px; @include ltr {
background-image: url('../images/sequence-nav/previous-icon.png');
}
// CASE: left to right layout // CASE: right to left layout
@include ltr { @include rtl {
background-image: url('../images/sequence-nav/previous-icon.png'); background-image: url('../images/sequence-nav/next-icon.png');
} }
}
// CASE: right to left layout &.button-next {
@include rtl { @include border-radius(0, 35px, 35px, 0);
background-image: url('../images/sequence-nav/next-icon.png'); right: 0;
} background-position: center 15px;
}
}
&.next { // CASE: left to right layout
@include right(-10px); @include ltr {
@include border-radius(0, 35px, 35px, 0); background-image: url('../images/sequence-nav/next-icon.png');
}
a { // CASE: right to left layout
@include margin-left(30px); @include rtl {
background-position: center 15px; background-image: url('../images/sequence-nav/previous-icon.png');
}
}
// CASE: left to right layout &:hover,
@include ltr { &:active {
background-image: url('../images/sequence-nav/next-icon.png');
}
// CASE: right to left layout
@include rtl {
background-image: url('../images/sequence-nav/previous-icon.png');
}
}
}
}
} }
body.touch-based-device & ol li a:hover p { &.disabled {
display: none; cursor: normal;
} }
} }
...@@ -326,83 +296,11 @@ nav.sequence-nav { ...@@ -326,83 +296,11 @@ nav.sequence-nav {
} }
nav.sequence-bottom { nav.sequence-bottom {
margin: lh(2) 0 0; position: relative;
width: 79px;
height: 1px;
margin: lh(2) auto;
text-align: center; text-align: center;
ul {
@include clearfix();
display: inline-block;
li {
@include float(left);
width: 50px;
height: 44px;
border: 1px solid $gray-l3;
@include linear-gradient(top, #eee, #ddd);
box-shadow: 0 1px 0 rgba(255, 255, 255, .7) inset;
&.prev, &.next {
margin-bottom: 0;
a {
background-position: center center;
background-repeat: no-repeat;
border: none;
display: block;
padding: lh(0.5) 4px;
text-indent: -9999px;
overflow: hidden;
@include transition(all .2s $ease-in-out-quad 0s);
&:hover, &:focus {
opacity: 0.5;
background-position: center 15px;
}
&.disabled {
opacity: 0.4;
}
}
}
&.prev {
@include border-radius(35px, 0, 0, 35px);
a {
background-position: center 15px;
// CASE: left to right layout
@include ltr {
background-image: url('../images/sequence-nav/previous-icon.png');
}
// CASE: right to left layout
@include rtl {
background-image: url('../images/sequence-nav/next-icon.png');
}
}
}
&.next {
@include border-radius(0, 35px, 35px, 0);
@include border-left(none);
a {
background-position: center 15px;
// CASE: left to right layout
@include ltr {
background-image: url('../images/sequence-nav/next-icon.png');
}
// CASE: right to left layout
@include rtl {
background-image: url('../images/sequence-nav/previous-icon.png');
}
}
}
}
}
} }
#seq_content { #seq_content {
......
common/lib/xmodule/xmodule/js/spec/sequence/display_spec.coffee
...@@ -40,11 +40,11 @@ xdescribe 'Sequence', -> ...@@ -40,11 +40,11 @@ xdescribe 'Sequence', ->
@sequence.toggleArrows() @sequence.toggleArrows()
it 'disable the previous button', -> it 'disable the previous button', ->
expect($('.sequence-nav-buttons .prev a')).toHaveClass 'disabled' expect($('.sequence-nav-button.button-previous')).toHaveClass 'disabled'
it 'enable the next button', -> it 'enable the next button', ->
expect($('.sequence-nav-buttons .next a')).not.toHaveClass 'disabled' expect($('.sequence-nav-button.button-next')).not.toHaveClass 'disabled'
expect($('.sequence-nav-buttons .next a')).toHandleWith 'click', @sequence.next expect($('.sequence-nav-button.button-next')).toHandleWith 'click', @sequence.next
describe 'when the middle tab is active', -> describe 'when the middle tab is active', ->
beforeEach -> beforeEach ->
...@@ -52,12 +52,12 @@ xdescribe 'Sequence', -> ...@@ -52,12 +52,12 @@ xdescribe 'Sequence', ->
@sequence.toggleArrows() @sequence.toggleArrows()
it 'enable the previous button', -> it 'enable the previous button', ->
expect($('.sequence-nav-buttons .prev a')).not.toHaveClass 'disabled' expect($('.sequence-nav-button.button-previous')).not.toHaveClass 'disabled'
expect($('.sequence-nav-buttons .prev a')).toHandleWith 'click', @sequence.previous expect($('.sequence-nav-button.button-previous')).toHandleWith 'click', @sequence.previous
it 'enable the next button', -> it 'enable the next button', ->
expect($('.sequence-nav-buttons .next a')).not.toHaveClass 'disabled' expect($('.sequence-nav-button.button-next')).not.toHaveClass 'disabled'
expect($('.sequence-nav-buttons .next a')).toHandleWith 'click', @sequence.next expect($('.sequence-nav-button.button-next')).toHandleWith 'click', @sequence.next
describe 'when the last tab is active', -> describe 'when the last tab is active', ->
beforeEach -> beforeEach ->
...@@ -65,11 +65,11 @@ xdescribe 'Sequence', -> ...@@ -65,11 +65,11 @@ xdescribe 'Sequence', ->
@sequence.toggleArrows() @sequence.toggleArrows()
it 'enable the previous button', -> it 'enable the previous button', ->
expect($('.sequence-nav-buttons .prev a')).not.toHaveClass 'disabled' expect($('.sequence-nav-button.button-previous')).not.toHaveClass 'disabled'
expect($('.sequence-nav-buttons .prev a')).toHandleWith 'click', @sequence.previous expect($('.sequence-nav-button.button-previous')).toHandleWith 'click', @sequence.previous
it 'disable the next button', -> it 'disable the next button', ->
expect($('.sequence-nav-buttons .next a')).toHaveClass 'disabled' expect($('.sequence-nav-button.button-next')).toHaveClass 'disabled'
describe 'render', -> describe 'render', ->
beforeEach -> beforeEach ->
...@@ -135,7 +135,7 @@ xdescribe 'Sequence', -> ...@@ -135,7 +135,7 @@ xdescribe 'Sequence', ->
jasmine.stubRequests() jasmine.stubRequests()
@sequence = new Sequence '1', 'sequence_1', @items, 'sequence', 2 @sequence = new Sequence '1', 'sequence_1', @items, 'sequence', 2
$.scrollTo 150 $.scrollTo 150
$('.sequence-nav-buttons .next a').click() $('.sequence-nav-button.button-next').click()
it 'log the next sequence event', -> it 'log the next sequence event', ->
expect(Logger.log).toHaveBeenCalledWith 'seq_next', old: 2, new: 3, id: '1' expect(Logger.log).toHaveBeenCalledWith 'seq_next', old: 2, new: 3, id: '1'
...@@ -151,7 +151,7 @@ xdescribe 'Sequence', -> ...@@ -151,7 +151,7 @@ xdescribe 'Sequence', ->
jasmine.stubRequests() jasmine.stubRequests()
@sequence = new Sequence '1', 'sequence_1', @items, 'sequence', 2 @sequence = new Sequence '1', 'sequence_1', @items, 'sequence', 2
$.scrollTo 150 $.scrollTo 150
$('.sequence-nav-buttons .prev a').click() $('.sequence-nav-button.button-previous').click()
it 'log the previous sequence event', -> it 'log the previous sequence event', ->
expect(Logger.log).toHaveBeenCalledWith 'seq_prev', old: 2, new: 1, id: '1' expect(Logger.log).toHaveBeenCalledWith 'seq_prev', old: 2, new: 1, id: '1'
......
common/lib/xmodule/xmodule/js/src/sequence/display.coffee
...@@ -72,22 +72,22 @@ class @Sequence ...@@ -72,22 +72,22 @@ class @Sequence
when 'done' then element.addClass('progress-done') when 'done' then element.addClass('progress-done')
toggleArrows: => toggleArrows: =>
@$('.sequence-nav-buttons a').unbind('click') @$('.sequence-nav-button').unbind('click')
if @contents.length == 0 if @contents.length == 0
@$('.sequence-nav-buttons .prev a').addClass('disabled').attr('aria-hidden', 'true') @$('.sequence-nav-button.button-previous').addClass('disabled').attr('disabled', true)
@$('.sequence-nav-buttons .next a').addClass('disabled').attr('aria-hidden', 'true') @$('.sequence-nav-button.button-next').addClass('disabled').attr('disabled', true)
return return
if @position == 1 if @position == 1
@$('.sequence-nav-buttons .prev a').addClass('disabled').attr('aria-hidden', 'true') @$('.sequence-nav-button.button-previous').addClass('disabled').attr('disabled', true)
else else
@$('.sequence-nav-buttons .prev a').removeClass('disabled').attr('aria-hidden', 'false').click(@previous) @$('.sequence-nav-button.button-previous').removeClass('disabled').removeAttr('disabled').click(@previous)
if @position == @contents.length if @position == @contents.length
@$('.sequence-nav-buttons .next a').addClass('disabled').attr('aria-hidden', 'true') @$('.sequence-nav-button.button-next').addClass('disabled').attr('disabled', true)
else else
@$('.sequence-nav-buttons .next a').removeClass('disabled').attr('aria-hidden', 'false').click(@next) @$('.sequence-nav-button.button-next').removeClass('disabled').removeAttr('disabled').click(@next)
render: (new_position) -> render: (new_position) ->
if @position != new_position if @position != new_position
......
common/lib/xmodule/xmodule/js/src/sequence/display/jquery.sequence.js
...@@ -3,7 +3,7 @@ var SequenceNav = function($element) { ...@@ -3,7 +3,7 @@ var SequenceNav = function($element) {
var $element = $element; var $element = $element;
var $wrapper = $element.find('.sequence-list-wrapper'); var $wrapper = $element.find('.sequence-list-wrapper');
var $list = $element.find('#sequence-list'); var $list = $element.find('#sequence-list');
var $arrows = $element.find('.sequence-nav-buttons'); var $arrows = $element.find('.sequence-nav-button');
var maxScroll = $list.width() - $wrapper.width() + 20; var maxScroll = $list.width() - $wrapper.width() + 20;
var $leftShadow = $('<div class="left-shadow"></div>'); var $leftShadow = $('<div class="left-shadow"></div>');
var $rightShadow = $('<div class="right-shadow"></div>'); var $rightShadow = $('<div class="right-shadow"></div>');
......
lms/templates/seq_module.html
<%! from django.utils.translation import ugettext as _ %> <%! from django.utils.translation import ugettext as _ %>
<div id="sequence_${element_id}" class="sequence" data-id="${item_id}" data-position="${position}" data-ajax-url="${ajax_url}" > <div id="sequence_${element_id}" class="sequence" data-id="${item_id}" data-position="${position}" data-ajax-url="${ajax_url}" >
<nav class="sequence-nav" aria-label="${_('Unit')}" > <div class="sequence-nav">
<ul class="sequence-nav-buttons"> <button class="sequence-nav-button button-previous">${_('Previous')}</button>
<li class="prev"><a role="button" href="#">${_('Previous')}</a></li> <nav class="sequence-list-wrapper" aria-label="${_('Unit')}">
</ul> <ol id="sequence-list">
<div class="sequence-list-wrapper">
<ol role="tablist" id="sequence-list">
% for idx, item in enumerate(items): % for idx, item in enumerate(items):
## TODO (vshnayder): add item.progress_detail either to the title or somewhere else. ## TODO (vshnayder): add item.progress_detail either to the title or somewhere else.
## Make sure it gets updated after ajax calls. ## Make sure it gets updated after ajax calls.
...@@ -29,12 +26,9 @@ ...@@ -29,12 +26,9 @@
</li> </li>
% endfor % endfor
</ol> </ol>
</div> </nav>
<button class="sequence-nav-button button-next">${_('Next')}</button>
<ul class="sequence-nav-buttons"> </div>
<li class="next"><a role="button" href="#">${_("Next")}</a></li>
</ul>
</nav>
% for idx, item in enumerate(items): % for idx, item in enumerate(items):
<div id="seq_contents_${idx}" <div id="seq_contents_${idx}"
...@@ -44,14 +38,11 @@ ...@@ -44,14 +38,11 @@
${item['content'] | h} ${item['content'] | h}
</div> </div>
% endfor % endfor
<div id="sr-is-focusable" tabindex="-1"></div>
<div id="seq_content"></div> <div id="seq_content"></div>
<nav class="sequence-bottom" aria-label="${_('Section')}"> <nav class="sequence-bottom" aria-label="${_('Section')}">
<ul class="sequence-nav-buttons"> <button class="sequence-nav-button button-previous">${_('Previous')}</button>
<li class="prev"><a role="button" href="#">${_("Previous")}</a></li> <button class="sequence-nav-button button-next">${_('Next')}</button>
<li class="next"><a role="button" href="#">${_("Next")}</a></li>
</ul>
</nav> </nav>
</div> </div>
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment