From f96d4b71429c02401d58301da53c1b9775b05b5c Mon Sep 17 00:00:00 2001 From: Christine Lytwynec Date: Thu, 27 Aug 2015 15:47:01 -0400 Subject: [PATCH 1/2] Manage focus on modal confirmation prompt --- .../common/js/components/views/feedback.js | 48 ++++++++++++- .../js/components/views/feedback_prompt.js | 9 +++ .../js/spec/components/feedback_spec.js | 69 ++++++++++++++++++- common/static/js_test_requirejs.yml | 1 + common/test/acceptance/pages/common/utils.py | 4 ++ .../static/teams/js/views/team_profile.js | 1 - 6 files changed, 129 insertions(+), 3 deletions(-) diff --git a/common/static/common/js/components/views/feedback.js b/common/static/common/js/components/views/feedback.js index d6f4cde749..0656b59458 100644 --- a/common/static/common/js/components/views/feedback.js +++ b/common/static/common/js/components/views/feedback.js @@ -6,6 +6,17 @@ "backbone", "text!common/templates/components/system-feedback.underscore"], function($, _, str, Backbone, systemFeedbackTemplate) { + var tabbable_elements = [ + "a[href]:not([tabindex='-1'])", + "area[href]:not([tabindex='-1'])", + "input:not([disabled]):not([tabindex='-1'])", + "select:not([disabled]):not([tabindex='-1'])", + "textarea:not([disabled]):not([tabindex='-1'])", + "button:not([disabled]):not([tabindex='-1'])", + "iframe:not([tabindex='-1'])", + "[tabindex]:not([tabindex='-1'])", + "[contentEditable=true]:not([tabindex='-1'])" + ]; var SystemFeedback = Backbone.View.extend({ options: { title: "", @@ -16,7 +27,8 @@ icon: true, // should we render an icon related to the message intent? closeIcon: true, // should we render a close button in the top right corner? minShown: 0, // length of time after this view has been shown before it can be hidden (milliseconds) - maxShown: Infinity // length of time after this view has been shown before it will be automatically hidden (milliseconds) + maxShown: Infinity, // length of time after this view has been shown before it will be automatically hidden (milliseconds) + outFocusElement: null // element to send focus to on hide /* Could also have an "actions" hash: here is an example demonstrating the expected structure. For each action, by default the framework @@ -65,6 +77,40 @@ return this; }, + inFocus: function() { + this.options.outFocusElement = this.options.outFocusElement || document.activeElement; + + // Set focus to the container. + this.$(".wrapper").first().focus(); + + + // Make tabs within the prompt loop rather than setting focus + // back to the main content of the page. + var tabbables = this.$(tabbable_elements.join()); + tabbables.on("keydown", function (event) { + // On tab backward from the first tabbable item in the prompt + if (event.which === 9 && event.shiftKey && event.target === tabbables.first()[0]) { + event.preventDefault(); + tabbables.last().focus(); + } + // On tab forward from the last tabbable item in the prompt + else if (event.which === 9 && !event.shiftKey && event.target === tabbables.last()[0]) { + event.preventDefault(); + tabbables.first().focus(); + } + }); + + return this; + }, + + outFocus: function() { + var tabbables = this.$(tabbable_elements.join()).off("keydown"); + if (this.options.outFocusElement) { + this.options.outFocusElement.focus(); + } + return this; + }, + // public API: show() and hide() show: function() { clearTimeout(this.hideTimeout); diff --git a/common/static/common/js/components/views/feedback_prompt.js b/common/static/common/js/components/views/feedback_prompt.js index 04a32da9c4..31c1e9b4ad 100644 --- a/common/static/common/js/components/views/feedback_prompt.js +++ b/common/static/common/js/components/views/feedback_prompt.js @@ -18,6 +18,15 @@ } // super() in Javascript has awkward syntax :( return SystemFeedbackView.prototype.render.apply(this, arguments); + }, + show: function() { + SystemFeedbackView.prototype.show.apply(this, arguments); + return this.inFocus(); + }, + + hide: function() { + SystemFeedbackView.prototype.hide.apply(this, arguments); + return this.outFocus(); } }); diff --git a/common/static/common/js/spec/components/feedback_spec.js b/common/static/common/js/spec/components/feedback_spec.js index 5ef3600204..a5e406fe3c 100644 --- a/common/static/common/js/spec/components/feedback_spec.js +++ b/common/static/common/js/spec/components/feedback_spec.js @@ -1,7 +1,8 @@ // Generated by CoffeeScript 1.6.1 (function() { - define(["jquery", "common/js/components/views/feedback", "common/js/components/views/feedback_notification", "common/js/components/views/feedback_alert", "common/js/components/views/feedback_prompt", "sinon"], function($, SystemFeedback, NotificationView, AlertView, PromptView, sinon) { + define(["jquery", "common/js/components/views/feedback", "common/js/components/views/feedback_notification", "common/js/components/views/feedback_alert", "common/js/components/views/feedback_prompt", "sinon", "jquery.simulate"], + function($, SystemFeedback, NotificationView, AlertView, PromptView, sinon) { var tpl; tpl = readFixtures('system-feedback.underscore'); beforeEach(function() { @@ -114,6 +115,72 @@ }); }); describe("PromptView", function() { + beforeEach(function() { + this.options = { + title: "Confirming Something", + message: "Are you sure you want to do this?", + actions: { + primary: { + text: "Yes, I'm sure.", + "class": "confirm-button", + }, + secondary: { + text: "Cancel", + "class": "cancel-button", + } + } + } + this.inFocusSpy = spyOn(PromptView.Confirmation.prototype, 'inFocus').andCallThrough(); + return this.outFocusSpy = spyOn(PromptView.Confirmation.prototype, 'outFocus').andCallThrough(); + }); + it("is focused on show", function() { + var view; + view = new PromptView.Confirmation(this.options).show(); + expect(this.inFocusSpy).toHaveBeenCalled(); + return waitsFor( + function() { return view.$('.wrapper-prompt:focus').length === 1; }, + "The modal should have focus", + 500 + ); + }); + it("is not focused on hide", function() { + var view; + view = new PromptView.Confirmation(this.options).hide(); + expect(this.outFocusSpy).toHaveBeenCalled(); + return waitsFor( + function() { return view.$('.wrapper-prompt:focus').length === 0; }, + "The modal should not have focus", + 500 + ); + }); + it("traps keyboard focus when moving forward", function() { + var view; + view = new PromptView.Confirmation(this.options).show(); + expect(this.inFocusSpy).toHaveBeenCalled(); + $('.action-secondary').first().simulate( + "keydown", + { keyCode: $.simulate.keyCode.TAB } + ); + return waitsFor( + function() { return view.$('.action-primary:focus').length === 1; }, + "The first action button should have focus", + 500 + ); + }); + it("traps keyboard focus when moving backward", function() { + var view; + view = new PromptView.Confirmation(this.options).show(); + expect(this.inFocusSpy).toHaveBeenCalled(); + $('.action-primary').first().simulate( + "keydown", + { keyCode: $.simulate.keyCode.TAB, shiftKey: true } + ); + return waitsFor( + function() { return view.$('.action-secondary:focus').length === 1; }, + "The last action button should have focus", + 500 + ); + }); return it("changes class on body", function() { var view; view = new PromptView.Confirmation({ diff --git a/common/static/js_test_requirejs.yml b/common/static/js_test_requirejs.yml index 391322c05c..c152e39617 100644 --- a/common/static/js_test_requirejs.yml +++ b/common/static/js_test_requirejs.yml @@ -31,6 +31,7 @@ lib_paths: - js/vendor/jquery.min.js - js/vendor/jasmine-jquery.js - js/vendor/jasmine-imagediff.js + - js/vendor/jquery.simulate.js - js/vendor/jquery.truncate.js - js/vendor/underscore-min.js - js/vendor/underscore.string.min.js diff --git a/common/test/acceptance/pages/common/utils.py b/common/test/acceptance/pages/common/utils.py index 8487933f76..bb03f211f9 100644 --- a/common/test/acceptance/pages/common/utils.py +++ b/common/test/acceptance/pages/common/utils.py @@ -56,6 +56,10 @@ def confirm_prompt(page, cancel=False, require_notification=None): cancel is True. """ page.wait_for_element_visibility('.prompt', 'Prompt is visible') + page.wait_for_element_visibility( + '.wrapper-prompt:focus', + 'Prompt is in focus' + ) confirmation_button_css = '.prompt .action-' + ('secondary' if cancel else 'primary') page.wait_for_element_visibility(confirmation_button_css, 'Confirmation button is visible') require_notification = (not cancel) if require_notification is None else require_notification diff --git a/lms/djangoapps/teams/static/teams/js/views/team_profile.js b/lms/djangoapps/teams/static/teams/js/views/team_profile.js index 0f49d7c934..b5ddfcf373 100644 --- a/lms/djangoapps/teams/static/teams/js/views/team_profile.js +++ b/lms/djangoapps/teams/static/teams/js/views/team_profile.js @@ -93,7 +93,6 @@ }); } ); - $('.wrapper-prompt').focus(); } }); From 2dc5b8e89cb82ecd500958249c0679f37aa0b8a9 Mon Sep 17 00:00:00 2001 From: Christine Lytwynec Date: Thu, 24 Sep 2015 16:25:49 -0400 Subject: [PATCH 2/2] make verifyElementInFocus a ViewHelpers method --- .../js/spec/components/feedback_spec.js | 28 ++++--------------- .../common/js/spec_helpers/view_helpers.js | 23 +++++++++++++-- 2 files changed, 27 insertions(+), 24 deletions(-) diff --git a/common/static/common/js/spec/components/feedback_spec.js b/common/static/common/js/spec/components/feedback_spec.js index a5e406fe3c..9ed52894ce 100644 --- a/common/static/common/js/spec/components/feedback_spec.js +++ b/common/static/common/js/spec/components/feedback_spec.js @@ -1,8 +1,8 @@ // Generated by CoffeeScript 1.6.1 (function() { - define(["jquery", "common/js/components/views/feedback", "common/js/components/views/feedback_notification", "common/js/components/views/feedback_alert", "common/js/components/views/feedback_prompt", "sinon", "jquery.simulate"], - function($, SystemFeedback, NotificationView, AlertView, PromptView, sinon) { + define(["jquery", "common/js/components/views/feedback", "common/js/components/views/feedback_notification", "common/js/components/views/feedback_alert", "common/js/components/views/feedback_prompt", 'common/js/spec_helpers/view_helpers', "sinon", "jquery.simulate"], + function($, SystemFeedback, NotificationView, AlertView, PromptView, ViewHelpers, sinon) { var tpl; tpl = readFixtures('system-feedback.underscore'); beforeEach(function() { @@ -137,21 +137,13 @@ var view; view = new PromptView.Confirmation(this.options).show(); expect(this.inFocusSpy).toHaveBeenCalled(); - return waitsFor( - function() { return view.$('.wrapper-prompt:focus').length === 1; }, - "The modal should have focus", - 500 - ); + return ViewHelpers.verifyElementInFocus(view, ".wrapper-prompt") }); it("is not focused on hide", function() { var view; view = new PromptView.Confirmation(this.options).hide(); expect(this.outFocusSpy).toHaveBeenCalled(); - return waitsFor( - function() { return view.$('.wrapper-prompt:focus').length === 0; }, - "The modal should not have focus", - 500 - ); + return ViewHelpers.verifyElementNotInFocus(view, ".wrapper-prompt") }); it("traps keyboard focus when moving forward", function() { var view; @@ -161,11 +153,7 @@ "keydown", { keyCode: $.simulate.keyCode.TAB } ); - return waitsFor( - function() { return view.$('.action-primary:focus').length === 1; }, - "The first action button should have focus", - 500 - ); + return ViewHelpers.verifyElementInFocus(view, ".action-primary") }); it("traps keyboard focus when moving backward", function() { var view; @@ -175,11 +163,7 @@ "keydown", { keyCode: $.simulate.keyCode.TAB, shiftKey: true } ); - return waitsFor( - function() { return view.$('.action-secondary:focus').length === 1; }, - "The last action button should have focus", - 500 - ); + return ViewHelpers.verifyElementInFocus(view, ".action-secondary") }); return it("changes class on body", function() { var view; diff --git a/common/static/common/js/spec_helpers/view_helpers.js b/common/static/common/js/spec_helpers/view_helpers.js index 32885ebc39..93b5827b89 100644 --- a/common/static/common/js/spec_helpers/view_helpers.js +++ b/common/static/common/js/spec_helpers/view_helpers.js @@ -10,7 +10,8 @@ define(["jquery", "common/js/components/views/feedback_notification", "common/js verifyFeedbackHidden, createNotificationSpy, verifyNotificationShowing, verifyNotificationHidden, createPromptSpy, confirmPrompt, inlineEdit, verifyInlineEditChange, installMockAnalytics, removeMockAnalytics, verifyPromptShowing, verifyPromptHidden, - clickDeleteItem, patchAndVerifyRequest, submitAndVerifyFormSuccess, submitAndVerifyFormError; + clickDeleteItem, patchAndVerifyRequest, submitAndVerifyFormSuccess, submitAndVerifyFormError, + verifyElementInFocus, verifyElementNotInFocus; installViewTemplates = function() { appendSetFixtures('
'); @@ -127,6 +128,22 @@ define(["jquery", "common/js/components/views/feedback_notification", "common/js verifyNotificationShowing(notificationSpy, /Saving/); }; + verifyElementInFocus = function(view, selector) { + waitsFor( + function() { return view.$(selector + ':focus').length === 1; }, + "element to have focus: " + selector, + 500 + ); + }; + + verifyElementNotInFocus = function(view, selector) { + waitsFor( + function() { return view.$(selector + ':focus').length === 0; }, + "element to not have focus: " + selector, + 500 + ); + }; + return { 'installViewTemplates': installViewTemplates, 'createNotificationSpy': createNotificationSpy, @@ -143,7 +160,9 @@ define(["jquery", "common/js/components/views/feedback_notification", "common/js 'clickDeleteItem': clickDeleteItem, 'patchAndVerifyRequest': patchAndVerifyRequest, 'submitAndVerifyFormSuccess': submitAndVerifyFormSuccess, - 'submitAndVerifyFormError': submitAndVerifyFormError + 'submitAndVerifyFormError': submitAndVerifyFormError, + 'verifyElementInFocus': verifyElementInFocus, + 'verifyElementNotInFocus': verifyElementNotInFocus }; }); }).call(this, define || RequireJS.define);