Merge pull request #9402 from edx/clytwynec/TNL-2153

Change component types and template links to buttons

cms/djangoapps/contentstore/features/component_settings_editor_helpers.py

@@ -67,20 +67,20 @@ def _click_advanced():
     world.wait_for_visible(tab2_css)
 
 
-def _find_matching_link(category, component_type):
+def _find_matching_button(category, component_type):
     """
-    Find the link with the specified text. There should be one and only one.
+    Find the button with the specified text. There should be one and only one.
     """
 
-    # The tab shows links for the given category
-    links = world.css_find('div.new-component-{} a'.format(category))
+    # The tab shows buttons for the given category
+    buttons = world.css_find('div.new-component-{} button'.format(category))
 
-    # Find the link whose text matches what you're looking for
-    matched_links = [link for link in links if link.text == component_type]
+    # Find the button whose text matches what you're looking for
+    matched_buttons = [btn for btn in buttons if btn.text == component_type]
 
     # There should be one and only one
-    assert_equal(len(matched_links), 1)
-    return matched_links[0]
+    assert_equal(len(matched_buttons), 1)
+    return matched_buttons[0]
 
 
 def click_component_from_menu(category, component_type, is_advanced):
@@ -100,7 +100,7 @@ def click_component_from_menu(category, component_type, is_advanced):
 
     # Retry this in case the list is empty because you tried too fast.
     link = world.retry_on_exception(
-        lambda: _find_matching_link(category, component_type),
+        lambda: _find_matching_button(category, component_type),
         ignored_exceptions=AssertionError
     )
 

cms/static/js/spec/views/pages/container_spec.js

@@ -552,7 +552,7 @@ define(["jquery", "underscore", "underscore.string", "common/js/spec_helpers/aja
                         var clickNewComponent;
 
                         clickNewComponent = function (index) {
-                            containerPage.$(".new-component .new-component-type a.single-template")[index].click();
+                            containerPage.$(".new-component .new-component-type button.single-template")[index].click();
                         };
 
                         it('Attaches a handler to new component button', function() {
@@ -598,7 +598,7 @@ define(["jquery", "underscore", "underscore.string", "common/js/spec_helpers/aja
                             var showTemplatePicker, verifyCreateHtmlComponent;
 
                             showTemplatePicker = function () {
-                                containerPage.$('.new-component .new-component-type a.multiple-templates')[0].click();
+                                containerPage.$('.new-component .new-component-type button.multiple-templates')[0].click();
                             };
 
                             verifyCreateHtmlComponent = function (test, templateIndex, expectedRequest) {
@@ -606,7 +606,7 @@ define(["jquery", "underscore", "underscore.string", "common/js/spec_helpers/aja
                                 renderContainerPage(test, mockContainerXBlockHtml);
                                 showTemplatePicker();
                                 xblockCount = containerPage.$('.studio-xblock-wrapper').length;
-                                containerPage.$('.new-component-html a')[templateIndex].click();
+                                containerPage.$('.new-component-html button')[templateIndex].click();
                                 EditHelpers.verifyXBlockRequest(requests, expectedRequest);
                                 AjaxHelpers.respondWithJson(requests, {"locator": "new_item"});
                                 respondWithHtml(mockXBlockHtml);

cms/static/js/views/components/add_xblock.js

@@ -6,10 +6,10 @@ define(["jquery", "underscore", "gettext", "js/views/baseview", "common/js/compo
     function ($, _, gettext, BaseView, ViewUtils, AddXBlockButton, AddXBlockMenu) {
         var AddXBlockComponent = BaseView.extend({
             events: {
-                'click .new-component .new-component-type a.multiple-templates': 'showComponentTemplates',
-                'click .new-component .new-component-type a.single-template': 'createNewComponent',
+                'click .new-component .new-component-type button.multiple-templates': 'showComponentTemplates',
+                'click .new-component .new-component-type button.single-template': 'createNewComponent',
                 'click .new-component .cancel-button': 'closeNewComponent',
-                'click .new-component-templates .new-component-template a': 'createNewComponent',
+                'click .new-component-templates .new-component-template .button-component': 'createNewComponent',
                 'click .new-component-templates .cancel-button': 'closeNewComponent'
             },
 
@@ -43,13 +43,17 @@ define(["jquery", "underscore", "gettext", "js/views/baseview", "common/js/compo
                 type = $(event.currentTarget).data('type');
                 this.$('.new-component').slideUp(250);
                 this.$('.new-component-' + type).slideDown(250);
+                this.$('.new-component-' + type + ' div').focus();
             },
 
             closeNewComponent: function(event) {
                 event.preventDefault();
                 event.stopPropagation();
+                type = $(event.currentTarget).data('type');
                 this.$('.new-component').slideDown(250);
                 this.$('.new-component-templates').slideUp(250);
+                this.$('ul.new-component-type li button[data-type=' + type + ']').focus();
+
             },
 
             createNewComponent: function(event) {

cms/static/sass/assets/_graphics.scss

@@ -13,40 +13,35 @@
 
 .large-advanced-icon {
 	display: inline-block;
-	width: 100px;
-	height: 60px;
-	margin-right: ($baseline/4);
+	width: ($baseline*3);
+	height: ($baseline*3);
 	background: url(../images/large-advanced-icon.png) center no-repeat;
 }
 
 .large-discussion-icon {
 	display: inline-block;
-	width: 100px;
-	height: 60px;
-	margin-right: ($baseline/4);
+	width: ($baseline*3);
+	height: ($baseline*3);
 	background: url(../images/large-discussion-icon.png) center no-repeat;
 }
 
 .large-html-icon {
 	display: inline-block;
-	width: 100px;
-	height: 60px;
-	margin-right: ($baseline/4);
+	width: ($baseline*3);
+	height: ($baseline*3);
 	background: url(../images/large-html-icon.png) center no-repeat;
 }
 
 .large-problem-icon {
 	display: inline-block;
-	width: 100px;
-	height: 60px;
-	margin-right: ($baseline/4);
+	width: ($baseline*3);
+	height: ($baseline*3);
 	background: url(../images/large-problem-icon.png) center no-repeat;
 }
 
 .large-video-icon {
 	display: inline-block;
-	width: 100px;
-	height: 60px;
-	margin-right: ($baseline/4);
+	width: ($baseline*3);
+	height: ($baseline*3);
 	background: url(../images/large-video-icon.png) center no-repeat;
 }

cms/static/sass/elements/_modules.scss

@@ -130,9 +130,10 @@
         width: ($baseline*5);
         height: ($baseline*5);
         margin-bottom: ($baseline/2);
+        box-shadow: 0 1px 1px $shadow, 0 1px 0 rgba(255, 255, 255, .4) inset;
         border: 1px solid $green-d2;
         border-radius: ($baseline/4);
-        box-shadow: 0 1px 1px $shadow, 0 1px 0 rgba(255, 255, 255, .4) inset;
+        padding: 0;
         background-color: $green-l1;
         text-align: center;
         color: $white;
@@ -195,16 +196,17 @@
       }
     }
 
-    a {
+  .button-component {
       @include clearfix();
       @include transition(none);
       @extend %t-demi-strong;
       display: block;
+      width: 100%;
       border: 0px;
       padding: 7px $baseline;
       background: $white;
       color: $gray-d3;
-
+      text-align: left;
 
       &:hover {
         @include transition(background-color $tmg-f2 linear 0s);

cms/templates/js/add-xblock-component-button.underscore

 <% if (type === 'advanced' || templates.length > 1) { %>
-<a href="#" class="multiple-templates add-xblock-component-button" data-type="<%= type %>">
+<button type="button" class="multiple-templates add-xblock-component-button" data-type="<%= type %>">
 <% } else { %>
-<a href="#" class="single-template add-xblock-component-button" data-type="<%= type %>" data-category="<%= templates[0].category %>">
+<button type="button" class="single-template add-xblock-component-button" data-type="<%= type %>" data-category="<%= templates[0].category %>">
 <% } %>
   <span class="large-template-icon large-<%= type %>-icon"></span>
+  <span class="sr"> <%= gettext("Add Component:") %></span>
   <span class="name"><%= display_name %></span>
-</a>
+</button>

cms/templates/js/add-xblock-component-menu-problem.underscore

-<div class="tab-group tabs">
-    <ul class="problem-type-tabs nav-tabs">
+<div class="tab-group tabs" tabindex="-1" role="dialog" aria-label="<%-
+    interpolate(
+        gettext('%(type)s Component Template Menu'),
+            {type: type},
+            true
+    )
+%>">
+    <ul class="problem-type-tabs nav-tabs" tabindex='-1'>
         <li class="current">
             <a class="link-tab" href="#tab1"><%= gettext("Common Problem Types") %></a>
         </li>
@@ -13,16 +19,16 @@
                 <% if (templates[i].tab == "common") { %>
                     <% if (!templates[i].boilerplate_name) { %>
                     <li class="editor-md empty">
-                        <a href="#" data-category="<%= templates[i].category %>">
+                        <button type="button" class="button-component" data-category="<%= templates[i].category %>">
                             <span class="name"><%= templates[i].display_name %></span>
-                        </a>
+                        </button>
                     </li>
                     <% } else { %>
                     <li class="editor-md">
-                        <a href="#" data-category="<%= templates[i].category %>"
+                        <button type="button" class="button-component" data-category="<%= templates[i].category %>"
                            data-boilerplate="<%= templates[i].boilerplate_name %>">
                             <span class="name"><%= templates[i].display_name %></span>
-                        </a>
+                        </button>
                     </li>
                     <% } %>
                 <% } %>
@@ -34,14 +40,14 @@
             <% for (var i = 0; i < templates.length; i++) { %>
                 <% if (templates[i].tab == "advanced") { %>
                 <li class="editor-manual">
-                    <a href="#" data-category="<%= templates[i].category %>"
+                    <button type="button" class="button-component" data-category="<%= templates[i].category %>"
                        data-boilerplate="<%= templates[i].boilerplate_name %>">
                         <span class="name"><%= templates[i].display_name %></span>
-                    </a>
+                    </button>
                 </li>
                 <% } %>
             <% } %>
         </ul>
     </div>
+    <button class="cancel-button" data-type="<%= type %>"><%= gettext("Cancel") %></button>
 </div>
-<a href="#" class="cancel-button"><%= gettext("Cancel") %></a>

cms/templates/js/add-xblock-component-menu.underscore

 <% if (type === 'advanced' || templates.length > 1) { %>
-    <div class="tab current" id="tab1">
+    <div class="tab current" id="tab1" tabindex="-1" role="dialog" aria-label="<%-
+        interpolate(
+            gettext('%(type)s Component Template Menu'),
+                {type: type},
+                true
+        )
+    %>">
         <ul class="new-component-template">
             <% for (var i = 0; i < templates.length; i++) { %>
                 <% if (!templates[i].boilerplate_name) { %>
                 <li class="editor-md empty">
-                    <a href="#" data-category="<%= templates[i].category %>">
+                    <button type="button" class="button-component" data-category="<%= templates[i].category %>">
                         <span class="name"><%= templates[i].display_name %></span>
-                    </a>
+                    </button>
                 </li>
                 <% } else { %>
                 <li class="editor-md">
-                    <a href="#" data-category="<%= templates[i].category %>"
+                    <button type="button" class="button-component" data-category="<%= templates[i].category %>"
                        data-boilerplate="<%= templates[i].boilerplate_name %>">
                         <span class="name"><%= templates[i].display_name %></span>
-                    </a>
+                    </button>
                 </li>
                 <% } %>
             <% } %>
         </ul>
+        <button class="cancel-button" data-type="<%= type %>"><%= gettext("Cancel") %></button>
     </div>
-    <a href="#" class="cancel-button"><%= gettext("Cancel") %></a>
 <% } %>

common/test/acceptance/pages/studio/container.py

@@ -305,7 +305,7 @@ class ContainerPage(PageObject):
         Returns:
             list
         """
-        css = '#tab{tab_index} a[data-category={category_type}] span'.format(
+        css = '#tab{tab_index} button[data-category={category_type}] span'.format(
             tab_index=tab_index,
             category_type=category_type
         )

common/test/acceptance/pages/studio/utils.py

@@ -72,7 +72,7 @@ def add_discussion(page, menu_index=0):
     placement within the page).
     """
     page.wait_for_component_menu()
-    click_css(page, 'a>span.large-discussion-icon', menu_index)
+    click_css(page, 'button>span.large-discussion-icon', menu_index)
 
 
 def add_advanced_component(page, menu_index, name):
@@ -84,7 +84,7 @@ def add_advanced_component(page, menu_index, name):
     """
     # Click on the Advanced icon.
     page.wait_for_component_menu()
-    click_css(page, 'a>span.large-advanced-icon', menu_index, require_notification=False)
+    click_css(page, 'button>span.large-advanced-icon', menu_index, require_notification=False)
 
     # This does an animation to hide the first level of buttons
     # and instead show the Advanced buttons that are available.
@@ -95,7 +95,7 @@ def add_advanced_component(page, menu_index, name):
     page.wait_for_element_visibility('.new-component-advanced', 'Advanced component menu is visible')
 
     # Now click on the component to add it.
-    component_css = 'a[data-category={}]'.format(name)
+    component_css = 'button[data-category={}]'.format(name)
     page.wait_for_element_visibility(component_css, 'Advanced component {} is visible'.format(name))
 
     # Adding some components, e.g. the Discussion component, will make an ajax call
@@ -123,7 +123,7 @@ def add_component(page, item_type, specific_type):
             'Wait for the add component menu to disappear'
         )
 
-        all_options = page.q(css='.new-component-{} ul.new-component-template li a span'.format(item_type))
+        all_options = page.q(css='.new-component-{} ul.new-component-template li button span'.format(item_type))
         chosen_option = all_options.filter(lambda el: el.text == specific_type).first
         chosen_option.click()
     wait_for_notification(page)
@@ -139,13 +139,13 @@ def add_html_component(page, menu_index, boilerplate=None):
     """
     # Click on the HTML icon.
     page.wait_for_component_menu()
-    click_css(page, 'a>span.large-html-icon', menu_index, require_notification=False)
+    click_css(page, 'button>span.large-html-icon', menu_index, require_notification=False)
 
     # Make sure that the menu of HTML components is visible before clicking
     page.wait_for_element_visibility('.new-component-html', 'HTML component menu is visible')
 
     # Now click on the component to add it.
-    component_css = 'a[data-category=html]'
+    component_css = 'button[data-category=html]'
     if boilerplate:
         component_css += '[data-boilerplate={}]'.format(boilerplate)
     else:

common/test/acceptance/pages/studio/video/video.py

@@ -30,7 +30,7 @@ CLASS_SELECTORS = {
 }
 
 BUTTON_SELECTORS = {
-    'create_video': 'a[data-category="video"]',
+    'create_video': 'button[data-category="video"]',
     'handout_download': '.video-handout.video-download-button a',
     'handout_download_editor': '.wrapper-comp-setting.file-uploader .download-action',
     'upload_asset': '.upload-action',