From 6b3e100cc1694900ba91457bc2529e27efbc470a Mon Sep 17 00:00:00 2001 From: Andy Armstrong Date: Tue, 20 May 2014 15:28:24 -0400 Subject: [PATCH] Allow editing of container xblocks/xmodules STUD-1312 --- CHANGELOG.rst | 2 + cms/djangoapps/contentstore/views/item.py | 28 +- cms/djangoapps/contentstore/views/preview.py | 7 +- .../views/tests/test_container_page.py | 6 +- .../views/tests/test_unit_page.py | 4 +- .../coffee/src/views/module_edit.coffee | 6 +- cms/static/js/models/xblock_info.js | 2 +- cms/static/js/spec/views/container_spec.js | 2 +- .../js/spec/views/pages/container_spec.js | 80 +++- cms/static/js/spec/views/unit_spec.js | 98 +++- cms/static/js/spec_helpers/edit_helpers.js | 10 +- cms/static/js/spec_helpers/modal_helpers.js | 17 +- cms/static/js/views/container.js | 5 +- cms/static/js/views/modals/edit_xblock.js | 15 +- cms/static/js/views/pages/container.js | 14 +- cms/static/sass/_base.scss | 1 + cms/static/sass/elements/_controls.scss | 1 + cms/static/sass/elements/_xblocks.scss | 73 ++- cms/static/sass/views/_container.scss | 43 +- cms/static/sass/views/_unit.scss | 52 ++- cms/templates/container.html | 13 +- cms/templates/container_xblock_component.html | 57 ++- .../js/mock/mock-container-page.underscore | 13 +- .../js/mock/mock-container-xblock.underscore | 436 +++++++++--------- .../mock-unit-page-child-container.underscore | 37 ++ .../mock-updated-container-xblock.underscore | 18 + cms/templates/studio_container_wrapper.html | 39 -- cms/templates/studio_xblock_wrapper.html | 80 ++-- cms/templates/widgets/sequence-edit.html | 51 -- .../xmodule/js/src/sequence/edit.coffee | 7 - .../xmodule/js/src/vertical/edit.coffee | 7 - .../xmodule/js/src/wrapper/edit.coffee | 10 - .../lib/xmodule/xmodule/split_test_module.py | 39 +- common/lib/xmodule/xmodule/studio_editable.py | 23 +- .../xmodule/tests/test_split_module.py | 41 +- .../xmodule/xmodule/tests/test_vertical.py | 11 + common/lib/xmodule/xmodule/vertical_module.py | 13 +- common/lib/xmodule/xmodule/wrapper_module.py | 4 - common/static/sass/_mixins.scss | 16 +- .../pages/studio/component_editor.py | 66 +++ .../test/acceptance/pages/studio/container.py | 25 +- common/test/acceptance/pages/studio/utils.py | 2 +- .../acceptance/tests/test_studio_container.py | 46 +- .../studio_render_children_view.html | 10 +- 44 files changed, 1016 insertions(+), 514 deletions(-) create mode 100644 cms/templates/js/mock/mock-unit-page-child-container.underscore create mode 100644 cms/templates/js/mock/mock-updated-container-xblock.underscore delete mode 100644 cms/templates/studio_container_wrapper.html delete mode 100644 common/lib/xmodule/xmodule/js/src/wrapper/edit.coffee create mode 100644 common/test/acceptance/pages/studio/component_editor.py diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 378ffcf10c..3c0e7c2265 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -14,6 +14,8 @@ Blades: Remove Video player outline. BLD-975. Blades: Fix Youtube regular expression in video player editor. BLD-967. +Studio: Support editing of containers. STUD-1312. + Blades: Fix displaying transcripts on touch devices. BLD-1033. Blades: Tolerance expressed in percentage now computes correctly. BLD-522. diff --git a/cms/djangoapps/contentstore/views/item.py b/cms/djangoapps/contentstore/views/item.py index d5d40fd277..bdbe64b444 100644 --- a/cms/djangoapps/contentstore/views/item.py +++ b/cms/djangoapps/contentstore/views/item.py @@ -199,25 +199,6 @@ def xblock_view_handler(request, usage_key_string, view_name): # change not authored by requestor but by xblocks. store.update_item(xblock, None) - - elif view_name == 'student_view' and xblock_has_own_studio_page(xblock): - context = { - 'runtime_type': 'studio', - 'container_view': False, - 'read_only': is_read_only, - 'root_xblock': xblock, - } - # For non-leaf xblocks on the unit page, show the special rendering - # which links to the new container page. - html = render_to_string('container_xblock_component.html', { - 'xblock_context': context, - 'xblock': xblock, - 'locator': usage_key, - }) - return JsonResponse({ - 'html': html, - 'resources': [], - }) elif view_name in (unit_views + container_views): is_container_view = (view_name in container_views) @@ -245,8 +226,15 @@ def xblock_view_handler(request, usage_key_string, view_name): # the component div. Note that the container view recursively adds headers # into the preview fragment, so we don't want to add another header here. if not is_container_view: - fragment.content = render_to_string('component.html', { + # For non-leaf xblocks, show the special rendering which links to the new container page. + if xblock_has_own_studio_page(xblock): + template = 'container_xblock_component.html' + else: + template = 'component.html' + fragment.content = render_to_string(template, { 'xblock_context': context, + 'xblock': xblock, + 'locator': usage_key, 'preview': fragment.content, 'label': xblock.display_name or xblock.scope_ids.block_type, }) diff --git a/cms/djangoapps/contentstore/views/preview.py b/cms/djangoapps/contentstore/views/preview.py index 87f1beb9a5..0a2139a80c 100644 --- a/cms/djangoapps/contentstore/views/preview.py +++ b/cms/djangoapps/contentstore/views/preview.py @@ -180,12 +180,7 @@ def _studio_wrap_xblock(xblock, view, frag, context, display_name_only=False): 'is_root': is_root, 'is_reorderable': is_reorderable, } - # For child xblocks with their own page, render a link to the page - if xblock_has_own_studio_page(xblock) and not is_root: - template = 'studio_container_wrapper.html' - else: - template = 'studio_xblock_wrapper.html' - html = render_to_string(template, template_context) + html = render_to_string('studio_xblock_wrapper.html', template_context) frag = wrap_fragment(frag, html) return frag diff --git a/cms/djangoapps/contentstore/views/tests/test_container_page.py b/cms/djangoapps/contentstore/views/tests/test_container_page.py index 3cb0ceb3b0..ea4c0fda51 100644 --- a/cms/djangoapps/contentstore/views/tests/test_container_page.py +++ b/cms/djangoapps/contentstore/views/tests/test_container_page.py @@ -137,8 +137,6 @@ class ContainerPageTestCase(StudioPageTestCase): """ empty_child_container = ItemFactory.create(parent_location=self.vertical.location, category='split_test', display_name='Split Test') - ItemFactory.create(parent_location=empty_child_container.location, - category='html', display_name='Split Child') self.validate_preview_html(empty_child_container, self.reorderable_child_view, can_reorder=False, can_edit=False, can_add=False) @@ -148,9 +146,7 @@ class ContainerPageTestCase(StudioPageTestCase): """ empty_child_container = ItemFactory.create(parent_location=self.vertical.location, category='split_test', display_name='Split Test') - ItemFactory.create(parent_location=empty_child_container.location, - category='html', display_name='Split Child') modulestore('draft').convert_to_draft(self.vertical.location) draft_empty_child_container = modulestore('draft').convert_to_draft(empty_child_container.location) self.validate_preview_html(draft_empty_child_container, self.reorderable_child_view, - can_reorder=True, can_edit=False, can_add=False) + can_reorder=True, can_edit=True, can_add=False) diff --git a/cms/djangoapps/contentstore/views/tests/test_unit_page.py b/cms/djangoapps/contentstore/views/tests/test_unit_page.py index f356cdbbda..569d7ab9d5 100644 --- a/cms/djangoapps/contentstore/views/tests/test_unit_page.py +++ b/cms/djangoapps/contentstore/views/tests/test_unit_page.py @@ -60,7 +60,7 @@ class UnitPageTestCase(StudioPageTestCase): ItemFactory.create(parent_location=child_container.location, category='html', display_name='grandchild') self.validate_preview_html(child_container, 'student_view', - can_reorder=True, can_edit=False, can_add=False) + can_reorder=True, can_edit=True, can_add=False) def test_draft_child_container_preview_html(self): """ @@ -74,4 +74,4 @@ class UnitPageTestCase(StudioPageTestCase): modulestore('draft').convert_to_draft(self.vertical.location) draft_child_container = modulestore('draft').get_item(child_container.location) self.validate_preview_html(draft_child_container, 'student_view', - can_reorder=True, can_edit=False, can_add=False) + can_reorder=True, can_edit=True, can_add=False) diff --git a/cms/static/coffee/src/views/module_edit.coffee b/cms/static/coffee/src/views/module_edit.coffee index 3c91647f61..1f093b3fdb 100644 --- a/cms/static/coffee/src/views/module_edit.coffee +++ b/cms/static/coffee/src/views/module_edit.coffee @@ -15,7 +15,11 @@ define ["jquery", "underscore", "gettext", "xblock/runtime.v1", @render() loadDisplay: -> - XBlock.initializeBlock(@$el.find('.xblock-student_view')) + # Not all components render an inline student view, e.g. child containers which + # instead render a link to a separate container page. + xblockElement = @$el.find('.xblock-student_view') + if xblockElement.length > 0 + XBlock.initializeBlock(xblockElement) createItem: (parent, payload, callback=->) -> payload.parent_locator = parent diff --git a/cms/static/js/models/xblock_info.js b/cms/static/js/models/xblock_info.js index 906a7b63bc..aa16604654 100644 --- a/cms/static/js/models/xblock_info.js +++ b/cms/static/js/models/xblock_info.js @@ -11,7 +11,7 @@ define(["backbone", "js/utils/module"], function(Backbone, ModuleUtils) { "is_container": null, "data": null, "metadata" : null, - "children": [] + "children": null } }); return XBlockInfo; diff --git a/cms/static/js/spec/views/container_spec.js b/cms/static/js/spec/views/container_spec.js index b0fe734a7f..c61d3bd114 100644 --- a/cms/static/js/spec/views/container_spec.js +++ b/cms/static/js/spec/views/container_spec.js @@ -11,7 +11,7 @@ define([ "jquery", "js/spec_helpers/create_sinon", "js/spec_helpers/view_helpers getDragHandle, dragComponentVertically, dragComponentAbove, verifyRequest, verifyNumReorderCalls, respondToRequest, notificationSpy, - rootLocator = 'testCourse/branch/draft/split_test/splitFFF', + rootLocator = 'locator-container', containerTestUrl = '/xblock/' + rootLocator, groupAUrl = "/xblock/locator-group-A", diff --git a/cms/static/js/spec/views/pages/container_spec.js b/cms/static/js/spec/views/pages/container_spec.js index 2227751cb6..b50666e961 100644 --- a/cms/static/js/spec/views/pages/container_spec.js +++ b/cms/static/js/spec/views/pages/container_spec.js @@ -7,6 +7,7 @@ define(["jquery", "underscore", "js/spec_helpers/create_sinon", "js/spec_helpers model, containerPage, requests, mockContainerPage = readFixtures('mock/mock-container-page.underscore'), mockContainerXBlockHtml = readFixtures('mock/mock-container-xblock.underscore'), + mockUpdatedContainerXBlockHtml = readFixtures('mock/mock-updated-container-xblock.underscore'), mockXBlockEditorHtml = readFixtures('mock/mock-xblock-editor.underscore'); beforeEach(function () { @@ -14,8 +15,8 @@ define(["jquery", "underscore", "js/spec_helpers/create_sinon", "js/spec_helpers appendSetFixtures(mockContainerPage); model = new XBlockInfo({ - id: 'testCourse/branch/draft/block/verticalFFF', - display_name: 'Test Unit', + id: 'locator-container', + display_name: 'Test Container', category: 'vertical' }); containerPage = new ContainerPage({ @@ -51,13 +52,14 @@ define(["jquery", "underscore", "js/spec_helpers/create_sinon", "js/spec_helpers }); }; - describe("Basic display", function() { + describe("Initial display", function() { it('can render itself', function() { renderContainerPage(mockContainerXBlockHtml, this); expect(containerPage.$el.select('.xblock-header')).toBeTruthy(); expect(containerPage.$('.wrapper-xblock')).not.toHaveClass('is-hidden'); expect(containerPage.$('.no-container-content')).toHaveClass('is-hidden'); }); + it('shows a loading indicator', function() { requests = create_sinon.requests(this); containerPage.render(); @@ -67,6 +69,66 @@ define(["jquery", "underscore", "js/spec_helpers/create_sinon", "js/spec_helpers }); }); + describe("Editing the container", function() { + var newDisplayName = 'New Display Name'; + + beforeEach(function () { + edit_helpers.installMockXBlock({ + data: "

Some HTML

", + metadata: { + display_name: newDisplayName + } + }); + }); + + afterEach(function() { + edit_helpers.uninstallMockXBlock(); + edit_helpers.cancelModalIfShowing(); + }); + + it('can edit itself', function() { + var editButtons, modalElement, + updatedTitle = 'Updated Test Container'; + renderContainerPage(mockContainerXBlockHtml, this); + + // Click the root edit button + editButtons = containerPage.$('.nav-actions .edit-button'); + editButtons.first().click(); + modalElement = edit_helpers.getModalElement(); + + // Expect a request to be made to show the studio view for the container + expect(lastRequest().url).toBe( + '/xblock/locator-container/studio_view' + ); + create_sinon.respondWithJson(requests, { + html: mockContainerXBlockHtml, + resources: [] + }); + expect(edit_helpers.isShowingModal()).toBeTruthy(); + + // Expect the correct title to be shown + expect(modalElement.find('.modal-window-title').text()).toBe('Editing: Test Container'); + + // Press the save button and respond with a success message to the save + edit_helpers.pressModalButton('.action-save'); + create_sinon.respondWithJson(requests, { }); + expect(edit_helpers.isShowingModal()).toBeFalsy(); + + // Expect the last request be to refresh the container page + expect(lastRequest().url).toBe( + '/xblock/locator-container/container_preview' + ); + create_sinon.respondWithJson(requests, { + html: mockUpdatedContainerXBlockHtml, + resources: [] + }); + + // Expect the title and breadcrumb to be updated + expect(containerPage.$('.page-header-title').text().trim()).toBe(updatedTitle); + expect(containerPage.$('.page-header .subtitle a').last().text().trim()).toBe(updatedTitle); + }); + }); + describe("Editing an xblock", function() { var newDisplayName = 'New Display Name'; @@ -87,10 +149,10 @@ define(["jquery", "underscore", "js/spec_helpers/create_sinon", "js/spec_helpers it('can show an edit modal for a child xblock', function() { var editButtons; renderContainerPage(mockContainerXBlockHtml, this); - editButtons = containerPage.$('.edit-button'); - // The container renders six mock xblocks, so there should be an equal number of edit buttons + editButtons = containerPage.$('.wrapper-xblock .edit-button'); + // The container should have rendered six mock xblocks expect(editButtons.length).toBe(6); - editButtons.first().click(); + editButtons[0].click(); // Make sure that the correct xblock is requested to be edited expect(lastRequest().url).toBe( '/xblock/locator-component-A1/studio_view' @@ -125,10 +187,10 @@ define(["jquery", "underscore", "js/spec_helpers/create_sinon", "js/spec_helpers var editButtons, modal, mockUpdatedXBlockHtml; mockUpdatedXBlockHtml = readFixtures('mock/mock-updated-xblock.underscore'); renderContainerPage(mockContainerXBlockHtml, this); - editButtons = containerPage.$('.edit-button'); - // The container renders six mock xblocks, so there should be an equal number of edit buttons + editButtons = containerPage.$('.wrapper-xblock .edit-button'); + // The container should have rendered six mock xblocks expect(editButtons.length).toBe(6); - editButtons.first().click(); + editButtons[0].click(); create_sinon.respondWithJson(requests, { html: mockXModuleEditor, resources: [] diff --git a/cms/static/js/spec/views/unit_spec.js b/cms/static/js/spec/views/unit_spec.js index 730b2eff59..f9d51f0246 100644 --- a/cms/static/js/spec/views/unit_spec.js +++ b/cms/static/js/spec/views/unit_spec.js @@ -1,7 +1,8 @@ define(["jquery", "underscore", "jasmine", "coffee/src/views/unit", "js/models/module_info", "js/spec_helpers/create_sinon", "js/spec_helpers/edit_helpers", "jasmine-stealth"], function ($, _, jasmine, UnitEditView, ModuleModel, create_sinon, edit_helpers) { - var requests, unitView, initialize, respondWithHtml, verifyComponents, i; + var requests, unitView, initialize, lastRequest, respondWithHtml, verifyComponents, i, + mockXBlockEditorHtml = readFixtures('mock/mock-xblock-editor.underscore'); respondWithHtml = function(html, requestIndex) { create_sinon.respondWithJson( @@ -13,6 +14,7 @@ define(["jquery", "underscore", "jasmine", "coffee/src/views/unit", "js/models/m initialize = function(test) { var mockXBlockHtml = readFixtures('mock/mock-unit-page-xblock.underscore'), + mockChildContainerHtml = readFixtures('mock/mock-unit-page-child-container.underscore'), model; requests = create_sinon.requests(test); model = new ModuleModel({ @@ -25,11 +27,13 @@ define(["jquery", "underscore", "jasmine", "coffee/src/views/unit", "js/models/m model: model }); - // Respond with renderings for the two xblocks in the unit + // Respond with renderings for the two xblocks in the unit (the second is itself a child container) respondWithHtml(mockXBlockHtml, 0); - respondWithHtml(mockXBlockHtml, 1); + respondWithHtml(mockChildContainerHtml, 1); }; + lastRequest = function() { return requests[requests.length - 1]; }; + verifyComponents = function (unit, locators) { var components = unit.$(".component"); expect(components.length).toBe(locators.length); @@ -174,5 +178,93 @@ define(["jquery", "underscore", "jasmine", "coffee/src/views/unit", "js/models/m test_link_disabled_during_ajax_call(draft_states[i]); } }); + + describe("Editing an xblock", function() { + var newDisplayName = 'New Display Name'; + + beforeEach(function () { + edit_helpers.installMockXBlock({ + data: "

Some HTML

", + metadata: { + display_name: newDisplayName + } + }); + }); + + afterEach(function() { + edit_helpers.uninstallMockXBlock(); + edit_helpers.cancelModalIfShowing(); + }); + + it('can show an edit modal for a child xblock', function() { + var editButtons; + initialize(this); + editButtons = unitView.$('.edit-button'); + // The container renders two mock xblocks + expect(editButtons.length).toBe(2); + editButtons[1].click(); + // Make sure that the correct xblock is requested to be edited + expect(lastRequest().url).toBe( + '/xblock/loc_2/studio_view' + ); + create_sinon.respondWithJson(requests, { + html: mockXBlockEditorHtml, + resources: [] + }); + expect(edit_helpers.isShowingModal()).toBeTruthy(); + }); + }); + + describe("Editing an xmodule", function() { + var mockXModuleEditor = readFixtures('mock/mock-xmodule-editor.underscore'), + newDisplayName = 'New Display Name'; + + beforeEach(function () { + edit_helpers.installMockXModule({ + data: "

Some HTML

", + metadata: { + display_name: newDisplayName + } + }); + }); + + afterEach(function() { + edit_helpers.uninstallMockXModule(); + edit_helpers.cancelModalIfShowing(); + }); + + it('can save changes to settings', function() { + var editButtons, modal, mockUpdatedXBlockHtml; + mockUpdatedXBlockHtml = readFixtures('mock/mock-updated-xblock.underscore'); + initialize(this); + editButtons = unitView.$('.edit-button'); + // The container renders two mock xblocks + expect(editButtons.length).toBe(2); + editButtons[1].click(); + create_sinon.respondWithJson(requests, { + html: mockXModuleEditor, + resources: [] + }); + + modal = $('.edit-xblock-modal'); + // Click on the settings tab + modal.find('.settings-button').click(); + // Change the display name's text + modal.find('.setting-input').text("Mock Update"); + // Press the save button + modal.find('.action-save').click(); + // Respond to the save + create_sinon.respondWithJson(requests, { + id: 'mock-id' + }); + + // Respond to the request to refresh + respondWithHtml(mockUpdatedXBlockHtml); + + // Verify that the xblock was updated + expect(unitView.$('.mock-updated-content').text()).toBe('Mock Update'); + }); + }); + }); }); diff --git a/cms/static/js/spec_helpers/edit_helpers.js b/cms/static/js/spec_helpers/edit_helpers.js index f88ce22b7c..869949b39f 100644 --- a/cms/static/js/spec_helpers/edit_helpers.js +++ b/cms/static/js/spec_helpers/edit_helpers.js @@ -9,11 +9,17 @@ define(["jquery", "underscore", "js/spec_helpers/create_sinon", "js/spec_helpers var installMockXBlock, uninstallMockXBlock, installMockXModule, uninstallMockXModule, mockComponentTemplates, installEditTemplates, showEditModal, verifyXBlockRequest; - installMockXBlock = function() { + installMockXBlock = function(mockResult) { window.MockXBlock = function(runtime, element) { - return { + var block = { runtime: runtime }; + if (mockResult) { + block.save = function() { + return mockResult; + }; + } + return block; }; }; diff --git a/cms/static/js/spec_helpers/modal_helpers.js b/cms/static/js/spec_helpers/modal_helpers.js index 1674132235..9267e74523 100644 --- a/cms/static/js/spec_helpers/modal_helpers.js +++ b/cms/static/js/spec_helpers/modal_helpers.js @@ -7,6 +7,7 @@ define(["jquery", "js/spec_helpers/view_helpers"], getModalElement, isShowingModal, hideModalIfShowing, + pressModalButton, cancelModal, cancelModalIfShowing; @@ -37,12 +38,16 @@ define(["jquery", "js/spec_helpers/view_helpers"], } }; - cancelModal = function(modal) { - var modalElement, cancelButton; + pressModalButton = function(selector, modal) { + var modalElement, button; modalElement = getModalElement(modal); - cancelButton = modalElement.find('.action-cancel:visible'); - expect(cancelButton.length).toBe(1); - cancelButton.click(); + button = modalElement.find(selector + ':visible'); + expect(button.length).toBe(1); + button.click(); + }; + + cancelModal = function(modal) { + pressModalButton('.action-cancel', modal); }; cancelModalIfShowing = function(modal) { @@ -52,9 +57,11 @@ define(["jquery", "js/spec_helpers/view_helpers"], }; return $.extend(view_helpers, { + 'getModalElement': getModalElement, 'installModalTemplates': installModalTemplates, 'isShowingModal': isShowingModal, 'hideModalIfShowing': hideModalIfShowing, + 'pressModalButton': pressModalButton, 'cancelModal': cancelModal, 'cancelModalIfShowing': cancelModalIfShowing }); diff --git a/cms/static/js/views/container.js b/cms/static/js/views/container.js index e29381e9b1..85b3dc7fee 100644 --- a/cms/static/js/views/container.js +++ b/cms/static/js/views/container.js @@ -1,6 +1,7 @@ define(["jquery", "underscore", "js/views/xblock", "js/utils/module", "gettext", "js/views/feedback_notification"], function ($, _, XBlockView, ModuleUtils, gettext, NotificationView) { var reorderableClass = '.reorderable-container', + sortableInitializedClass = '.ui-sortable', studioXBlockWrapperClass = '.studio-xblock-wrapper'; var ContainerView = XBlockView.extend({ @@ -8,7 +9,7 @@ define(["jquery", "underscore", "js/views/xblock", "js/utils/module", "gettext", xblockReady: function () { XBlockView.prototype.xblockReady.call(this); var reorderableContainer = this.$(reorderableClass), - alreadySortable = this.$('.ui-sortable'), + alreadySortable = this.$(sortableInitializedClass), newParent, oldParent, self = this; @@ -113,7 +114,7 @@ define(["jquery", "underscore", "js/views/xblock", "js/utils/module", "gettext", }, refresh: function() { - this.$(reorderableClass).sortable('refresh'); + this.$(sortableInitializedClass).sortable('refresh'); } }); diff --git a/cms/static/js/views/modals/edit_xblock.js b/cms/static/js/views/modals/edit_xblock.js index 294770d4ad..d946593e82 100644 --- a/cms/static/js/views/modals/edit_xblock.js +++ b/cms/static/js/views/modals/edit_xblock.js @@ -111,13 +111,9 @@ define(["jquery", "underscore", "gettext", "js/views/modals/base_modal", }, getTitle: function() { - var displayName = this.xblockElement.find('.xblock-header .header-details').text().trim(); - // If not found, try the old unit page style rendering + var displayName = this.xblockInfo.get('display_name'); if (!displayName) { - displayName = this.xblockElement.find('.component-header').text().trim(); - if (!displayName) { - displayName = gettext('Component'); - } + displayName = gettext('Component'); } return interpolate(gettext("Editing: %(title)s"), { title: displayName }, true); }, @@ -180,13 +176,16 @@ define(["jquery", "underscore", "gettext", "js/views/modals/base_modal", findXBlockInfo: function(xblockWrapperElement, defaultXBlockInfo) { var xblockInfo = defaultXBlockInfo, - xblockElement; + xblockElement, + displayName; if (xblockWrapperElement.length > 0) { xblockElement = xblockWrapperElement.find('.xblock'); + displayName = xblockWrapperElement.find('.xblock-header .header-details').text().trim(); xblockInfo = new XBlockInfo({ id: xblockWrapperElement.data('locator'), courseKey: xblockWrapperElement.data('course-key'), - category: xblockElement.data('block-type') + category: xblockElement.data('block-type'), + display_name: displayName }); } return xblockInfo; diff --git a/cms/static/js/views/pages/container.js b/cms/static/js/views/pages/container.js index b1715ea425..4a1d7fb54a 100644 --- a/cms/static/js/views/pages/container.js +++ b/cms/static/js/views/pages/container.js @@ -46,6 +46,7 @@ define(["jquery", "underscore", "gettext", "js/views/feedback_notification", } else { noContentElement.removeClass('is-hidden'); } + self.refreshTitle(); loadingElement.addClass('is-hidden'); self.delegateEvents(); } @@ -60,6 +61,12 @@ define(["jquery", "underscore", "gettext", "js/views/feedback_notification", return this.xblockView.model.urlRoot; }, + refreshTitle: function() { + var title = this.$('.xblock-header .header-details span').first().text().trim(); + this.$('.page-header-title').text(title); + this.$('.page-header .subtitle a').last().text(title); + }, + onXBlockRefresh: function(xblockView) { this.addButtonActions(xblockView.$el); this.xblockView.refresh(); @@ -177,10 +184,9 @@ define(["jquery", "underscore", "gettext", "js/views/feedback_notification", */ refreshXBlock: function(xblockElement) { var parentElement = xblockElement.parent(), - rootLocator = this.xblockView.model.id, - xblockLocator = xblockElement.data('locator'); - if (xblockLocator === rootLocator) { - this.render(); + rootLocator = this.xblockView.model.id; + if (xblockElement.length === 0 || xblockElement.data('locator') === rootLocator) { + this.render({ }); } else if (parentElement.hasClass('reorderable-container')) { this.refreshChildXBlock(xblockElement); } else { diff --git a/cms/static/sass/_base.scss b/cms/static/sass/_base.scss index 831c306f97..0e60236977 100644 --- a/cms/static/sass/_base.scss +++ b/cms/static/sass/_base.scss @@ -334,6 +334,7 @@ p, ul, ol, dl { .navigation-link { @extend %cont-truncated; display: inline-block; + vertical-align: bottom; // correct for extra padding in FF max-width: 250px; &.navigation-current { diff --git a/cms/static/sass/elements/_controls.scss b/cms/static/sass/elements/_controls.scss index 3e2a423a78..6af4514079 100644 --- a/cms/static/sass/elements/_controls.scss +++ b/cms/static/sass/elements/_controls.scss @@ -230,6 +230,7 @@ vertical-align: middle; .action-button { + @include transition(all $tmg-f3 linear 0s); display: block; border-radius: 3px; padding: ($baseline/4) ($baseline/2); diff --git a/cms/static/sass/elements/_xblocks.scss b/cms/static/sass/elements/_xblocks.scss index 198706a8e7..5fbb582658 100644 --- a/cms/static/sass/elements/_xblocks.scss +++ b/cms/static/sass/elements/_xblocks.scss @@ -15,11 +15,8 @@ } // UI: xblock header - .xblock-header { + .xblock-header-primary { @include box-sizing(border-box); - @include ui-flexbox(); - @extend %ui-align-center-flex; - justify-content: space-between; border-bottom: 1px solid $gray-l4; border-radius: ($baseline/5) ($baseline/5) 0 0; min-height: ($baseline*2.5); @@ -28,17 +25,73 @@ .header-details { @extend %cont-truncated; - @extend %ui-justify-left-flex; - @include ui-flexbox(); - width: flex-grid(6,12); + display: inline-block; + width: 50%; vertical-align: middle; } .header-actions { - @include ui-flexbox(); - @extend %ui-justify-right-flex; - width: flex-grid(6,12); + display: inline-block; + width: 49%; vertical-align: middle; + text-align: right; + } + } + + .xblock-header-secondary { + overflow: hidden; + border-top: 1px solid $gray-l3; + background-color: $gray-l5; + padding: ($baseline/2) $baseline; + + .meta-info { + display: inline-block; + vertical-align: middle; + width: 65%; + font-style: italic; + color: $gray; + } + + .actions-list { + width: 34%; + display: inline-block; + vertical-align: middle; + text-align: right; + + .action-item { + display: inline-block; + + .action-button { + @include transition(all $tmg-f3 linear 0s); + display: block; + width: auto; + height: ($baseline*1.5); + border-radius: 3px; + padding: 3px ($baseline/2) 0 ($baseline/2); + color: $gray-l1; + + &:hover { + background-color: $blue; + color: $gray-l6; + } + + .action-button-text { + display: inline-block; + vertical-align: middle; + padding: 0 1px; + text-transform: uppercase; + } + + &.delete-button:hover { + background-color: $gray-l1; + } + } + + [class^="icon-"] { + display: inline-block; + vertical-align: middle; + } + } } } } diff --git a/cms/static/sass/views/_container.scss b/cms/static/sass/views/_container.scss index 1c31180631..e2f6a8ae29 100644 --- a/cms/static/sass/views/_container.scss +++ b/cms/static/sass/views/_container.scss @@ -12,13 +12,42 @@ .mast { border-bottom: none; padding-bottom: 0; + + .page-header { + @extend %t-title; + @include font-size(28); + @include line-height(32); + + .subtitle .navigation-link { + color: $gray; + + &:hover { + color: $blue; + } + } + } } .wrapper-mast .mast.has-navigation .nav-actions { - bottom: -($baseline*.75); - .nav-item .button { + .nav-item { + .edit-button { + @include blue-button; + @extend %t-action4; + padding: ($baseline/4) ($baseline/2); + text-align: center; + + .action-button-text { + display: inline-block; + vertical-align: middle; + } + + [class^="icon-"] { + display: inline-block; + vertical-align: middle; + } + } } } @@ -140,6 +169,10 @@ body.view-container .content-primary { } .xblock-header { + display: block; + } + + .xblock-header-primary { @include ui-flexbox(); margin-bottom: 0; border-bottom: none; @@ -168,6 +201,10 @@ body.view-container .content-primary { } .xblock-header { + display: block; + } + + .xblock-header-primary { display: flex; margin-bottom: 0; border-bottom: 1px solid $gray-l4; @@ -183,7 +220,7 @@ body.view-container .content-primary { // STATE: xBlock containers styled as rows. &.xblock-type-container { - .xblock-header { + .xblock-header-primary { margin-bottom: 0; border-bottom: 0; border-radius: ($baseline/5); diff --git a/cms/static/sass/views/_unit.scss b/cms/static/sass/views/_unit.scss index ea08dfabc6..2f8b3d664b 100644 --- a/cms/static/sass/views/_unit.scss +++ b/cms/static/sass/views/_unit.scss @@ -750,13 +750,15 @@ body.unit { body.unit { - .component-actions { + .component-actions, + .xblock-header-secondary .actions-list { .action-item { display: inline-block; margin: ($baseline/4) 0 ($baseline/4) ($baseline/4); .action-button { + @include transition(all $tmg-f3 linear 0s); display: block; padding: 0 $baseline/2; width: auto; @@ -770,10 +772,10 @@ body.unit { } .action-button-text { - padding-left: 1px; - vertical-align: bottom; + display: inline-block; + vertical-align: middle; + padding: 0 1px; text-transform: uppercase; - line-height: 17px; } &.delete-button:hover { @@ -783,7 +785,7 @@ body.unit { [class^="icon-"] { display: inline-block; - vertical-align: bottom; + vertical-align: middle; } } } @@ -1128,7 +1130,7 @@ body.unit .xblock-type-container { } } - .xblock-header { + .xblock-header-primary { border-bottom: 0; border-radius: ($baseline/5); @@ -1178,6 +1180,44 @@ body.unit .component.editing { } } +// UI: special case xblock with no preview, ex. experiment blocks + +body.unit .component { + + .wrapper-component-action-header.nopreview { + position: relative; + border-bottom: 0; + } + + .xblock-header-secondary { + overflow: hidden; + border-top: 1px solid $gray-l3; + background-color: $gray-l5; + padding: ($baseline/2); + + .meta-info { + display: inline-block; + vertical-align: middle; + width: 65%; + padding-left: ($baseline/4); + font-style: italic; + color: $gray; + } + + .actions-list { + display: inline-block; + vertical-align: middle; + width: 33%; + text-align: right; + + .action-item { + margin: 0; + } + } + } +} + + body.view-unit .main-column .unit-body, body.view-container { diff --git a/cms/templates/container.html b/cms/templates/container.html index 2c08afa880..c19dbefc74 100644 --- a/cms/templates/container.html +++ b/cms/templates/container.html @@ -54,9 +54,9 @@ main_xblock_info = {
-
+

- + % for ancestor in ancestor_xblocks: <% ancestor_url = xblock_studio_url(ancestor) @@ -68,11 +68,20 @@ main_xblock_info = { % endfor ${xblock.display_name_with_default | h} + ${xblock.display_name_with_default | h}

diff --git a/cms/templates/container_xblock_component.html b/cms/templates/container_xblock_component.html index c7f436c1b3..13ee1c0721 100644 --- a/cms/templates/container_xblock_component.html +++ b/cms/templates/container_xblock_component.html @@ -4,24 +4,41 @@ from contentstore.views.helpers import xblock_studio_url %> <%namespace name='static' file='static_content.html'/> -
-
-
- ${xblock.display_name_with_default} -
-
- -
-
+
+
+ ${xblock.display_name_with_default} +
+
+
+
${_('This block contains multiple components.')}
+ +
+ diff --git a/cms/templates/js/mock/mock-container-page.underscore b/cms/templates/js/mock/mock-container-page.underscore index 5be3e75e4d..8a9a100fc9 100644 --- a/cms/templates/js/mock/mock-container-page.underscore +++ b/cms/templates/js/mock/mock-container-page.underscore @@ -3,15 +3,22 @@

- + Unit 1 - Nested Vertical Test + Test Container + Test Container

@@ -22,7 +29,7 @@
-
+
diff --git a/cms/templates/js/mock/mock-unit-page-child-container.underscore b/cms/templates/js/mock/mock-unit-page-child-container.underscore new file mode 100644 index 0000000000..43924f0683 --- /dev/null +++ b/cms/templates/js/mock/mock-unit-page-child-container.underscore @@ -0,0 +1,37 @@ +
+
+ Test Child Container +
+ +
+
+
This block contains multiple components.
+ +
+ diff --git a/cms/templates/js/mock/mock-updated-container-xblock.underscore b/cms/templates/js/mock/mock-updated-container-xblock.underscore new file mode 100644 index 0000000000..50a56efddc --- /dev/null +++ b/cms/templates/js/mock/mock-updated-container-xblock.underscore @@ -0,0 +1,18 @@ +
+
+
+ Updated Test Container +
+
+
    +
+
+
+
+
+
+
    +
+
+
diff --git a/cms/templates/studio_container_wrapper.html b/cms/templates/studio_container_wrapper.html deleted file mode 100644 index 79ac5f18fa..0000000000 --- a/cms/templates/studio_container_wrapper.html +++ /dev/null @@ -1,39 +0,0 @@ -<%! -from django.utils.translation import ugettext as _ -from contentstore.views.helpers import xblock_studio_url -%> -<%namespace name='static' file='static_content.html'/> - -% if is_reorderable: -
  • -% else: -
    -% endif -
    -
    -
    - ${xblock.display_name_with_default} -
    -
    - -
    -
    -
    -% if is_reorderable: -
    • -% else: -
    -% endif diff --git a/cms/templates/studio_xblock_wrapper.html b/cms/templates/studio_xblock_wrapper.html index 3483f515f9..5e2d6a9364 100644 --- a/cms/templates/studio_xblock_wrapper.html +++ b/cms/templates/studio_xblock_wrapper.html @@ -1,4 +1,14 @@ -<%! from django.utils.translation import ugettext as _ %> +<%! +from django.utils.translation import ugettext as _ +from contentstore.views.helpers import xblock_studio_url +%> + +<% +xblock_url = xblock_studio_url(xblock) +show_inline = xblock.has_children and not xblock_url +section_class = "level-nesting" if show_inline else "level-element" +collapsible_class = "is-collapsible" if xblock.has_children else "" +%> % if not is_root: % if is_reorderable: @@ -7,16 +17,13 @@
    % endif - <% - section_class = "level-nesting" if xblock.has_children else "level-element" - collapsible_class = "is-collapsible" if xblock.has_children else "" - %>
    % endif -
    +
    +
    - % if xblock.has_children: + % if show_inline: ${_('Expand or Collapse')} @@ -26,39 +33,56 @@
    -
    +
    + % if xblock_url and not is_root: +
    +
    ${_('This block contains multiple components.')}
    + +
    + % endif + +% if is_root or not xblock_url:
    ${content}
    +% endif % if not is_root: diff --git a/cms/templates/widgets/sequence-edit.html b/cms/templates/widgets/sequence-edit.html index 546633ee6e..4838e8a538 100644 --- a/cms/templates/widgets/sequence-edit.html +++ b/cms/templates/widgets/sequence-edit.html @@ -1,54 +1,3 @@
    -
    -
      -
    • -

      Sort:

      - -
      • - -
    • -

      Filter:

      - - More -
      • -
    • - -
      • -
    -
    - -
    -
    -
      -
    1. -
        - % for child in module.get_children(): -
      1. - ${child.display_name_with_default} - handle -
        2. - %endfor -
      -
      2. - -
    -
    -
    <%include file="metadata-edit.html" />
    - diff --git a/common/lib/xmodule/xmodule/js/src/sequence/edit.coffee b/common/lib/xmodule/xmodule/js/src/sequence/edit.coffee index 33942bc97d..1856266697 100644 --- a/common/lib/xmodule/xmodule/js/src/sequence/edit.coffee +++ b/common/lib/xmodule/xmodule/js/src/sequence/edit.coffee @@ -1,9 +1,2 @@ class @SequenceDescriptor extends XModule.Descriptor - constructor: (@element) -> - @$tabs = $(@element).find("#sequence-list") - @$tabs.sortable( - update: (event, ui) => @update() - ) - save: -> - children: $('#sequence-list li a', @element).map((idx, el) -> $(el).data('id')).toArray() diff --git a/common/lib/xmodule/xmodule/js/src/vertical/edit.coffee b/common/lib/xmodule/xmodule/js/src/vertical/edit.coffee index 8dbf8e2550..09bec3bef5 100644 --- a/common/lib/xmodule/xmodule/js/src/vertical/edit.coffee +++ b/common/lib/xmodule/xmodule/js/src/vertical/edit.coffee @@ -1,9 +1,2 @@ class @VerticalDescriptor extends XModule.Descriptor - constructor: (@element) -> - @$items = $(@element).find(".vert-mod") - @$items.sortable( - update: (event, ui) => @update() - ) - save: -> - children: $('.vert-mod div', @element).map((idx, el) -> $(el).data('id')).toArray() diff --git a/common/lib/xmodule/xmodule/js/src/wrapper/edit.coffee b/common/lib/xmodule/xmodule/js/src/wrapper/edit.coffee deleted file mode 100644 index 801b09d168..0000000000 --- a/common/lib/xmodule/xmodule/js/src/wrapper/edit.coffee +++ /dev/null @@ -1,10 +0,0 @@ -class @WrapperDescriptor extends XModule.Descriptor - constructor: (@element) -> - console.log 'WrapperDescriptor' - @$items = $(@element).find(".vert-mod") - @$items.sortable( - update: (event, ui) => @update() - ) - - save: -> - children: $('.vert-mod div', @element).map((idx, el) -> $(el).data('id')).toArray() diff --git a/common/lib/xmodule/xmodule/split_test_module.py b/common/lib/xmodule/xmodule/split_test_module.py index 40a772bcf2..f0c7d67240 100644 --- a/common/lib/xmodule/xmodule/split_test_module.py +++ b/common/lib/xmodule/xmodule/split_test_module.py @@ -8,12 +8,13 @@ from webob import Response from xmodule.progress import Progress from xmodule.seq_module import SequenceDescriptor +from xmodule.studio_editable import StudioEditableModule from xmodule.x_module import XModule, module_attr from lxml import etree from xblock.core import XBlock -from xblock.fields import Scope, Integer, ReferenceValueDict +from xblock.fields import Scope, Integer, String, ReferenceValueDict from xblock.fragment import Fragment log = logging.getLogger('edx.' + __name__) @@ -23,6 +24,13 @@ class SplitTestFields(object): """Fields needed for split test module""" has_children = True + display_name = String( + display_name="Display Name", + help="This name appears in the horizontal navigation at the top of the page.", + scope=Scope.settings, + default="Experiment Block" + ) + user_partition_id = Integer( help="Which user partition is used for this test", scope=Scope.content @@ -45,7 +53,7 @@ class SplitTestFields(object): @XBlock.needs('user_tags') # pylint: disable=abstract-method @XBlock.wants('partitions') -class SplitTestModule(SplitTestFields, XModule): +class SplitTestModule(SplitTestFields, XModule, StudioEditableModule): """ Show the user the appropriate child. Uses the ExperimentState API to figure out which child to show. @@ -177,21 +185,10 @@ class SplitTestModule(SplitTestFields, XModule): Renders the Studio preview by rendering each child so that they can all be seen and edited. """ fragment = Fragment() - contents = [] - - for child in self.descriptor.get_children(): - rendered_child = self.runtime.get_module(child).render('student_view', context) - fragment.add_frag_resources(rendered_child) - - contents.append({ - 'id': child.location.to_deprecated_string(), - 'content': rendered_child.content - }) - - fragment.add_content(self.system.render_template('vert_module.html', { - 'items': contents - })) - + # Only render the children when this block is being shown as the container + root_xblock = context.get('root_xblock') + if root_xblock and root_xblock.location == self.location: + self.render_children(context, fragment, can_reorder=False) return fragment def student_view(self, context): @@ -296,3 +293,11 @@ class SplitTestDescriptor(SplitTestFields, SequenceDescriptor): makes it use module.get_child_descriptors(). """ return True + + @property + def non_editable_metadata_fields(self): + non_editable_fields = super(SplitTestDescriptor, self).non_editable_metadata_fields + non_editable_fields.extend([ + SplitTestDescriptor.due, + ]) + return non_editable_fields diff --git a/common/lib/xmodule/xmodule/studio_editable.py b/common/lib/xmodule/xmodule/studio_editable.py index 799dbf1103..81284cd6e1 100644 --- a/common/lib/xmodule/xmodule/studio_editable.py +++ b/common/lib/xmodule/xmodule/studio_editable.py @@ -5,25 +5,34 @@ Mixin to support editing in Studio. class StudioEditableModule(object): """ - Helper methods for supporting Studio editing of xblocks. + Helper methods for supporting Studio editing of xblocks/xmodules. + + This class is only intended to be used with an XModule, as it assumes the existence of + self.descriptor and self.system. """ - def render_reorderable_children(self, context, fragment): + def render_children(self, context, fragment, can_reorder=False, can_add=False, view_name='student_view'): """ - Renders children with the appropriate HTML structure for drag and drop. + Renders the children of the module with HTML appropriate for Studio. If can_reorder is True, + then the children will be rendered to support drag and drop. """ contents = [] - for child in self.get_display_items(): - context['reorderable_items'].add(child.location) - rendered_child = child.render('student_view', context) + for child in self.descriptor.get_children(): # pylint: disable=E1101 + if can_reorder: + context['reorderable_items'].add(child.location) + child_module = self.system.get_module(child) # pylint: disable=E1101 + rendered_child = child_module.render(view_name, context) fragment.add_frag_resources(rendered_child) contents.append({ + 'id': child.location.to_deprecated_string(), 'content': rendered_child.content }) - fragment.add_content(self.system.render_template("studio_render_children_view.html", { + fragment.add_content(self.system.render_template("studio_render_children_view.html", { # pylint: disable=E1101 'items': contents, 'xblock_context': context, + 'can_add': can_add, + 'can_reorder': can_reorder, })) diff --git a/common/lib/xmodule/xmodule/tests/test_split_module.py b/common/lib/xmodule/xmodule/tests/test_split_module.py index 05223800f1..35a7f7b595 100644 --- a/common/lib/xmodule/xmodule/tests/test_split_module.py +++ b/common/lib/xmodule/xmodule/tests/test_split_module.py @@ -43,7 +43,7 @@ class SplitTestModuleTest(XModuleXmlImportTest): xml.HtmlFactory(parent=split_test, url_name='split_test_cond1', text='HTML FOR GROUP 1') self.course = self.process_xml(course) - course_seq = self.course.get_children()[0] + self.course_sequence = self.course.get_children()[0] self.module_system = get_test_system() def get_module(descriptor): @@ -71,7 +71,7 @@ class SplitTestModuleTest(XModuleXmlImportTest): ) self.module_system._services['partitions'] = self.partitions_service # pylint: disable=protected-access - self.split_test_module = course_seq.get_children()[0] + self.split_test_module = self.course_sequence.get_children()[0] self.split_test_module.bind_for_student(self.module_system, self.split_test_module._field_data) # pylint: disable=protected-access @ddt.data(('0', 'split_test_cond0'), ('1', 'split_test_cond1')) @@ -147,3 +147,40 @@ class SplitTestModuleTest(XModuleXmlImportTest): self.assertEquals(fields.get('user_partition_id'), '0') self.assertIsNotNone(fields.get('group_id_to_child')) self.assertEquals(len(children), 2) + + def test_render_studio_view(self): + """ + Test the rendering of the Studio view. + """ + + # The split_test module should render both its groups when it is the root + reorderable_items = set() + context = { + 'runtime_type': 'studio', + 'container_view': True, + 'reorderable_items': reorderable_items, + 'root_xblock': self.split_test_module, + } + html = self.module_system.render(self.split_test_module, 'student_view', context).content + self.assertIn('HTML FOR GROUP 0', html) + self.assertIn('HTML FOR GROUP 1', html) + + # When rendering as a child, it shouldn't render either of its groups + reorderable_items = set() + context = { + 'runtime_type': 'studio', + 'container_view': True, + 'reorderable_items': reorderable_items, + 'root_xblock': self.course_sequence, + } + html = self.module_system.render(self.split_test_module, 'student_view', context).content + self.assertNotIn('HTML FOR GROUP 0', html) + self.assertNotIn('HTML FOR GROUP 1', html) + + def test_settings(self): + """ + Test the settings configuration. + """ + non_editable_metadata_fields = self.split_test_module.non_editable_metadata_fields + self.assertIn(SplitTestDescriptor.due, non_editable_metadata_fields) + self.assertNotIn(SplitTestDescriptor.display_name, non_editable_metadata_fields) diff --git a/common/lib/xmodule/xmodule/tests/test_vertical.py b/common/lib/xmodule/xmodule/tests/test_vertical.py index 9d1e792734..600a37d224 100644 --- a/common/lib/xmodule/xmodule/tests/test_vertical.py +++ b/common/lib/xmodule/xmodule/tests/test_vertical.py @@ -54,9 +54,20 @@ class VerticalModuleTestCase(BaseVerticalModuleTest): """ Test the rendering of the Studio view """ + # Vertical shouldn't render children on the unit page + context = { + 'runtime_type': 'studio', + 'container_view': False, + } + html = self.module_system.render(self.vertical, 'student_view', context).content + self.assertNotIn(self.test_html_1, html) + self.assertNotIn(self.test_html_2, html) + + # Vertical should render reorderable children on the container page reorderable_items = set() context = { 'runtime_type': 'studio', + 'container_view': True, 'reorderable_items': reorderable_items, } html = self.module_system.render(self.vertical, 'student_view', context).content diff --git a/common/lib/xmodule/xmodule/vertical_module.py b/common/lib/xmodule/xmodule/vertical_module.py index c75d76a283..cfb8081bea 100644 --- a/common/lib/xmodule/xmodule/vertical_module.py +++ b/common/lib/xmodule/xmodule/vertical_module.py @@ -30,7 +30,10 @@ class VerticalModule(VerticalFields, XModule, StudioEditableModule): Renders the Studio preview view, which supports drag and drop. """ fragment = Fragment() - self.render_reorderable_children(context, fragment) + # For the container page we want the full drag-and-drop, but for unit pages we want + # a more concise version that appears alongside the "View =>" link. + if context.get('container_view'): + self.render_children(context, fragment, can_reorder=True, can_add=True) return fragment def render_view(self, context, template_name): @@ -82,3 +85,11 @@ class VerticalDescriptor(VerticalFields, SequenceDescriptor): # TODO (victor): Does this need its own definition_to_xml method? Otherwise it looks # like verticals will get exported as sequentials... + + @property + def non_editable_metadata_fields(self): + non_editable_fields = super(VerticalDescriptor, self).non_editable_metadata_fields + non_editable_fields.extend([ + VerticalDescriptor.due, + ]) + return non_editable_fields diff --git a/common/lib/xmodule/xmodule/wrapper_module.py b/common/lib/xmodule/xmodule/wrapper_module.py index d675f102bf..96546c3628 100644 --- a/common/lib/xmodule/xmodule/wrapper_module.py +++ b/common/lib/xmodule/xmodule/wrapper_module.py @@ -20,7 +20,3 @@ class WrapperDescriptor(VerticalDescriptor): module_class = WrapperModule has_children = True - - js = {'coffee': [resource_string(__name__, 'js/src/vertical/edit.coffee')]} - js_module_name = "VerticalDescriptor" - diff --git a/common/static/sass/_mixins.scss b/common/static/sass/_mixins.scss index 69e8cc200a..c96ff8657a 100644 --- a/common/static/sass/_mixins.scss +++ b/common/static/sass/_mixins.scss @@ -54,19 +54,19 @@ // extends - justify-content right for display:flex alignment in older browsers %ui-justify-right-flex { - -webkit-box-pack: end; - -moz-box-pack: end; - -ms-flex-pack: end; - -webkit-justify-content: end; + -webkit-box-pack: flex-end; + -moz-box-pack: flex-end; + -ms-flex-pack: flex-end; + -webkit-justify-content: flex-end; justify-content: flex-end; } // extends - justify-content left for display:flex alignment in older browsers %ui-justify-left-flex { - -webkit-box-pack: start; - -moz-box-pack: start; - -ms-flex-pack: start; - -webkit-justify-content: start; + -webkit-box-pack: flex-start; + -moz-box-pack: flex-start; + -ms-flex-pack: flex-start; + -webkit-justify-content: flex-start; justify-content: flex-start; } diff --git a/common/test/acceptance/pages/studio/component_editor.py b/common/test/acceptance/pages/studio/component_editor.py new file mode 100644 index 0000000000..57990b93b1 --- /dev/null +++ b/common/test/acceptance/pages/studio/component_editor.py @@ -0,0 +1,66 @@ +from bok_choy.page_object import PageObject +from selenium.webdriver.common.keys import Keys +from selenium.webdriver.common.action_chains import ActionChains +from utils import click_css + + +class ComponentEditorView(PageObject): + """ + A :class:`.PageObject` representing the rendered view of a component editor. + + This class assumes that the editor is our default editor as displayed for xmodules. + """ + BODY_SELECTOR = '.xblock-editor' + + def __init__(self, browser, locator): + """ + Args: + browser (selenium.webdriver): The Selenium-controlled browser that this page is loaded in. + locator (str): The locator that identifies which xblock this :class:`.xblock-editor` relates to. + """ + super(ComponentEditorView, self).__init__(browser) + self.locator = locator + + def is_browser_on_page(self): + return self.q(css='{}[data-locator="{}"]'.format(self.BODY_SELECTOR, self.locator)).present + + def _bounded_selector(self, selector): + """ + Return `selector`, but limited to this particular `ComponentEditorView` context + """ + return '{}[data-locator="{}"] {}'.format( + self.BODY_SELECTOR, + self.locator, + selector + ) + + def url(self): + """ + Returns None because this is not directly accessible via URL. + """ + return None + + def get_setting_entry_index(self, label): + """ + Returns the index of the setting entry with given label (display name) within the Settings modal. + """ + # TODO: will need to handle tabbed "Settings" in future (current usage is in vertical, only shows Settings. + setting_labels = self.q(css=self._bounded_selector('.metadata_edit .wrapper-comp-setting .setting-label')) + for index, setting in enumerate(setting_labels): + if setting.text == label: + return index + return None + + def set_field_value_and_save(self, label, value): + """ + Set the field with given label (display name) to the specified value, and presses Save. + """ + index = self.get_setting_entry_index(label) + elem = self.q(css=self._bounded_selector('.metadata_edit div.wrapper-comp-setting input.setting-input'))[index] + # Click in the field, delete the value there. + action = ActionChains(self.browser).click(elem) + for _x in range(0, len(elem.get_attribute('value'))): + action = action.send_keys(Keys.BACKSPACE) + # Send the new text, then Tab to move to the next field (so change event is triggered). + action.send_keys(value).send_keys(Keys.TAB).perform() + click_css(self, 'a.action-save') diff --git a/common/test/acceptance/pages/studio/container.py b/common/test/acceptance/pages/studio/container.py index 70d7f8db56..82baa3b673 100644 --- a/common/test/acceptance/pages/studio/container.py +++ b/common/test/acceptance/pages/studio/container.py @@ -3,7 +3,7 @@ Container page in Studio """ from bok_choy.page_object import PageObject -from bok_choy.promise import Promise +from bok_choy.promise import Promise, EmptyPromise from . import BASE_URL from selenium.webdriver.common.action_chains import ActionChains @@ -15,15 +15,24 @@ class ContainerPage(PageObject): """ Container page in Studio """ + NAME_SELECTOR = 'a.navigation-current' - def __init__(self, browser, unit_locator): + def __init__(self, browser, locator): super(ContainerPage, self).__init__(browser) - self.unit_locator = unit_locator + self.locator = locator @property def url(self): """URL to the container page for an xblock.""" - return "{}/container/{}".format(BASE_URL, self.unit_locator) + return "{}/container/{}".format(BASE_URL, self.locator) + + @property + def name(self): + titles = self.q(css=self.NAME_SELECTOR).text + if titles: + return titles[0] + else: + return None def is_browser_on_page(self): @@ -91,6 +100,14 @@ class ContainerPage(PageObject): # Click the confirmation dialog button click_css(self, 'a.button.action-primary', 0) + def edit(self): + self.q(css='.edit-button').first.click() + EmptyPromise( + lambda: self.q(css='.xblock-studio_view').present, + 'Wait for the Studio editor to be present' + ).fulfill() + + return self class XBlockWrapper(PageObject): diff --git a/common/test/acceptance/pages/studio/utils.py b/common/test/acceptance/pages/studio/utils.py index 5ccce44e48..33bac1e8c1 100644 --- a/common/test/acceptance/pages/studio/utils.py +++ b/common/test/acceptance/pages/studio/utils.py @@ -5,7 +5,7 @@ from bok_choy.promise import Promise from selenium.webdriver.common.action_chains import ActionChains -def click_css(page, css, source_index, require_notification=True): +def click_css(page, css, source_index=0, require_notification=True): """ Click the button/link with the given css and index on the specified page (subclass of PageObject). diff --git a/common/test/acceptance/tests/test_studio_container.py b/common/test/acceptance/tests/test_studio_container.py index b74c85826c..1300a6274d 100644 --- a/common/test/acceptance/tests/test_studio_container.py +++ b/common/test/acceptance/tests/test_studio_container.py @@ -1,11 +1,15 @@ """ Acceptance tests for Studio related to the container page. """ + from ..pages.studio.auto_auth import AutoAuthPage from ..pages.studio.overview import CourseOutlinePage from ..fixtures.course import CourseFixture, XBlockFixtureDesc from .helpers import UniqueCourseTest +from ..pages.studio.component_editor import ComponentEditorView + +from unittest import skip class ContainerBase(UniqueCourseTest): @@ -85,13 +89,17 @@ class ContainerBase(UniqueCourseTest): ).install() def go_to_container_page(self, make_draft=False): + unit = self.go_to_unit_page(make_draft) + container = unit.components[0].go_to_container() + return container + + def go_to_unit_page(self, make_draft=False): self.outline.visit() subsection = self.outline.section('Test Section').subsection('Test Subsection') unit = subsection.toggle_expand().unit('Test Unit').go_to() if make_draft: unit.edit_draft() - container = unit.components[0].go_to_container() - return container + return unit def verify_ordering(self, container, expected_orderings): xblocks = container.xblocks @@ -131,6 +139,7 @@ class DragAndDropTest(ContainerBase): expected_ordering ) + @skip("Sporadically drags outside of the Group.") def test_reorder_in_group(self): """ Drag Group A Item 2 before Group A Item 1. @@ -303,3 +312,36 @@ class DeleteComponentTest(ContainerBase): {self.group_b: [self.group_b_item_1, self.group_b_item_2]}, {self.group_empty: []}] self.delete_and_verify(self.group_a_item_1_action_index, expected_ordering) + + +class EditContainerTest(ContainerBase): + """ + Tests of editing a container. + """ + __test__ = True + + def modify_display_name_and_verify(self, component): + """ + Helper method for changing a display name. + """ + modified_name = 'modified' + self.assertNotEqual(component.name, modified_name) + component.edit() + component_editor = ComponentEditorView(self.browser, component.locator) + component_editor.set_field_value_and_save('Display Name', modified_name) + self.assertEqual(component.name, modified_name) + + def test_edit_container_on_unit_page(self): + """ + Test the "edit" button on a container appearing on the unit page. + """ + unit = self.go_to_unit_page(make_draft=True) + component = unit.components[0] + self.modify_display_name_and_verify(component) + + def test_edit_container_on_container_page(self): + """ + Test the "edit" button on a container appearing on the container page. + """ + container = self.go_to_container_page(make_draft=True) + self.modify_display_name_and_verify(container) diff --git a/lms/templates/studio_render_children_view.html b/lms/templates/studio_render_children_view.html index 0e220ce117..34be5f3ff6 100644 --- a/lms/templates/studio_render_children_view.html +++ b/lms/templates/studio_render_children_view.html @@ -1,8 +1,12 @@ -
      +% if can_reorder: +
        +% endif % for item in items: ${item['content']} % endfor -
      -% if not xblock_context['read_only']: +% if can_reorder: +
    +% endif +% if can_add and not xblock_context['read_only']:
    % endif