Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
82 changes: 76 additions & 6 deletions lms/static/js/my_courses_dropdown.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,79 @@
$(document).ready(function () {
$('a.dropdown').toggle(function() {
$('ul.dropdown-menu').addClass("expanded");
$('a.dropdown').addClass("active");
// define variables for code legibility
var dropdownMenuToggle = $('a.dropdown');
var dropdownMenu = $('ul.dropdown-menu');
var menuItems = dropdownMenu.find('a');

// bind menu toggle click for later use
dropdownMenuToggle.toggle(function() {
dropdownMenu.addClass("expanded").find('a').first().focus();
dropdownMenuToggle.addClass("active").attr("aria-expanded", "true");
}, function() {
$('ul.dropdown-menu').removeClass("expanded");
$('a.dropdown').removeClass("active");
dropdownMenu.removeClass("expanded");
dropdownMenuToggle.removeClass("active").attr("aria-expanded", "false").focus();

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

when the menu is closed, focus is automatically moved to menu toggle arrow.

});
});

//catch keypresses when focused on dropdownMenuToggle (we only care about spacebar keypresses here)
dropdownMenuToggle.on('keydown', function(event){
// if space key pressed
if ( event.which == 32) {
dropdownMenuToggle.click();
event.preventDefault();
}
});

//catch keypresses when inside dropdownMenu (we want to catch spacebar; escape; up arrow or shift+tab; and down arrow or tab)
dropdownMenu.on('keydown', function(event){
catchKeyPress($(this), event);
});

function catchKeyPress(object, event) {
// get currently focused item
var focusedItem = jQuery(':focus');

// get the number of focusable items
var numberOfMenuItems = menuItems.length

// get the index of the currently focused item
var focusedItemIndex = menuItems.index(focusedItem);

// var to store next focused item index
var itemToFocusIndex;

// if space key pressed
if ( event.which == 32) {
dropdownMenuToggle.click();
event.preventDefault();
}

// if escape key pressed
if (event.which == 27) {
dropdownMenuToggle.click();
event.preventDefault();
}

// if up arrow key pressed or shift+tab
if (event.which == 38 || (event.which == 9 && event.shiftKey)) {
// if first item go to last
if (focusedItemIndex === 0) {
menuItems.last().focus();
} else {
itemToFocusIndex = focusedItemIndex - 1;
menuItems.get(itemToFocusIndex).focus();
}
event.preventDefault();
}

// if down arrow key pressed or tab key
if (event.which == 40 || event.which == 9) {
// if last item go to first
if (focusedItemIndex == numberOfMenuItems - 1) {
menuItems.first().focus();
} else {
itemToFocusIndex = focusedItemIndex + 1;
menuItems.get(itemToFocusIndex).focus();
}
event.preventDefault();
}
}
});
6 changes: 3 additions & 3 deletions lms/templates/navigation.html
Original file line number Diff line number Diff line change
Expand Up @@ -72,12 +72,12 @@ <h2><span class="provider">${course.display_org_with_default | h}:</span> ${cour
</a>
</li>
<li class="primary">
<a href="#" class="dropdown"><span class="sr">${_("More options dropdown")}</span> &#9662;</a>
<ul class="dropdown-menu">
<a href="#" class="dropdown" aria-haspopup="true" aria-expanded="false"><span class="sr">${_("More options dropdown")}</span> &#9662;</a>

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

added aria attrs for screenreader use

<ul class="dropdown-menu" aria-label="More Options" role="menu">

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

added aria labels and role to have screenreaders read out helpful information

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Just a nitpick - why do you make comments in the pull request rather than commenting your code? If you don't think the code stands on its own, such that in your pull request you need to comment it, your code probably needs comments within it. The people maintaining and updating this code will almost certainly not have reviewed your pull request.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Sure @sarina I can definitely comment the code more. Is there any part of the code that you think needs more information? I try to add reasoning behind changes as github comments, but they don't seem like things that need to be in the code, if that makes sense...

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I just accidentally clicked on this PR and noticed a ton of inline comments from the PR author and just wanted to nitpick, and make sure you understand that it's ok to add comments (the more the better in my opinion).

<%block name="navigation_dropdown_menu_links" >
<li><a href="${marketing_link('FAQ')}">${_("Help")}</a></li>
</%block>
<li><a href="${reverse('logout')}">${_("Log Out")}</a></li>
<li role="presentation"><a href="${reverse('logout')}" role="menuitem">${_("Log Out")}</a></li>

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

added roles for a11y

</ul>
</li>
</ol>
Expand Down