Skip to content

E
edx-platform
Commit 2dcff3e9 by Andy Armstrong
Options

Merge pull request #3361 from edx/andya/rc-fix-duplicate-buttons 

Fix duplicate save and cancel buttons in Studio
parents f1eefc11 1b2d1858
Showing with 216 additions and 131 deletions
cms/djangoapps/contentstore/views/item.py
......@@ -246,10 +246,6 @@ def xblock_view_handler(request, package_id, view_name, tag=None, branch=None, v
fragment.content = render_to_string('component.html', {
'preview': fragment.content,
'label': component.display_name or component.scope_ids.block_type,
# Native XBlocks are responsible for persisting their own data,
# so they are also responsible for providing save/cancel buttons.
'show_save_cancel': isinstance(component, xmodule.x_module.XModuleDescriptor),
})
else:
raise Http404
......
cms/djangoapps/contentstore/views/tests/test_item.py
......@@ -765,34 +765,3 @@ class TestComponentHandler(TestCase):
self.descriptor.handle = create_response
self.assertEquals(component_handler(self.request, self.usage_id, 'dummy_handler').status_code, status_code)
@ddt.ddt
class TestNativeXBlock(ItemTest):
"""
Test a "native" XBlock (not an XModule shim).
"""
@ddt.data(('problem', True), ('acid', False))
@ddt.unpack
def test_save_cancel_buttons(self, category, include_buttons):
"""
Native XBlocks handle their own persistence, so Studio
should not render Save/Cancel buttons for them.
"""
# Create the XBlock
resp = self.create_xblock(category=category)
self.assertEqual(resp.status_code, 200)
native_loc = json.loads(resp.content)['locator']
# Render the XBlock
view_url = '/xblock/{locator}/student_view'.format(locator=native_loc)
resp = self.client.get(view_url, HTTP_ACCEPT='application/json')
self.assertEqual(resp.status_code, 200)
# Check that the save and cancel buttons are hidden for native XBlocks,
# but shown for XModule shim XBlocks
resp_html = json.loads(resp.content)['html']
assert_func = self.assertIn if include_buttons else self.assertNotIn
assert_func('save-button', resp_html)
assert_func('cancel-button', resp_html)
cms/static/coffee/spec/views/module_edit_spec.coffee
......@@ -25,7 +25,6 @@ define ["jquery", "js/spec_helpers/edit_helpers", "coffee/src/views/module_edit"
</section>
</li>
</ul>
<div class="edit-xblock-modal"/>
"""
edit_helpers.installEditTemplates(true);
spyOn($, 'ajax').andReturn(@moduleData)
......
cms/static/coffee/src/views/module_edit.coffee
......@@ -47,7 +47,6 @@ define ["jquery", "underscore", "gettext", "xblock/runtime.v1",
clickEditButton: (event) ->
event.preventDefault()
modal = new EditXBlockModal({
el: $('.edit-xblock-modal'),
view: 'student_view'
});
modal.edit(this.$el, self.model, { refresh: _.bind(@render, this) })
cms/static/coffee/src/xblock/cms.runtime.v1.coffee
......@@ -49,30 +49,32 @@ define [
# Finished saving, so hide the "Saving..." notification
else if data.state == 'end'
@_hideAlerts()
# Hide the editor *after* we finish saving in case there are validation
# errors that the user needs to correct.
@_hideEditor()
# Notify the modal that the save has completed so that it can hide itself
# and then refresh the xblock.
if @modal
@modal.onSave()
$('.component.editing').removeClass('editing')
@savingNotification.hide()
else if name == 'edit-modal-shown'
@modal = data
else if name == 'edit-modal-hidden'
@modal = null
else if name == 'cancel'
@_hideEditor()
@_hideAlerts()
if @modal
@modal.cancel()
else if name == 'error'
if 'msg' of data
@alert.options.message = data.msg
@alert.show()
_hideEditor: () ->
# This will close all open component editors, which works
# if we assume that <= 1 are open at a time.
el = $('.component.editing')
el.removeClass('editing')
el.find('.component-editor').slideUp(150)
ModalUtils.hideModalCover()
_hideAlerts: () ->
# Hide any alerts that are being shown
if @alert.options.shown
@alert.hide()
cms/static/js/spec/views/modals/edit_xblock_spec.js
......@@ -5,9 +5,9 @@ define(["jquery", "underscore", "js/spec_helpers/create_sinon", "js/spec_helpers
describe("EditXBlockModal", function() {
var model, modal, showModal;
showModal = function(requests, mockHtml) {
showModal = function(requests, mockHtml, options) {
var xblockElement = $('.xblock');
return edit_helpers.showEditModal(requests, xblockElement, model, mockHtml);
return edit_helpers.showEditModal(requests, xblockElement, model, mockHtml, options);
};
beforeEach(function () {
......@@ -45,6 +45,13 @@ define(["jquery", "underscore", "js/spec_helpers/create_sinon", "js/spec_helpers
expect(edit_helpers.isShowingModal(modal)).toBeFalsy();
});
it('does not show the "Save" button', function() {
var requests = create_sinon.requests(this);
modal = showModal(requests, mockXBlockEditorHtml);
expect(modal.$('.action-save')).not.toBeVisible();
expect(modal.$('.action-cancel').text()).toBe('OK');
});
it('shows the correct title', function() {
var requests = create_sinon.requests(this);
modal = showModal(requests, mockXBlockEditorHtml);
......@@ -56,6 +63,43 @@ define(["jquery", "underscore", "js/spec_helpers/create_sinon", "js/spec_helpers
modal = showModal(requests, mockXBlockEditorHtml);
expect(modal.$('.editor-modes a').length).toBe(0);
});
it('hides itself and refreshes after save notification', function() {
var requests = create_sinon.requests(this),
refreshed = false,
refresh = function() {
refreshed = true;
};
modal = showModal(requests, mockXBlockEditorHtml, { refresh: refresh });
modal.runtime.notify('save', { state: 'start' });
modal.runtime.notify('save', { state: 'end' });
expect(edit_helpers.isShowingModal(modal)).toBeFalsy();
expect(refreshed).toBeTruthy();
});
it('hides itself and does not refresh after cancel notification', function() {
var requests = create_sinon.requests(this),
refreshed = false,
refresh = function() {
refreshed = true;
};
modal = showModal(requests, mockXBlockEditorHtml, { refresh: refresh });
modal.runtime.notify('cancel');
expect(edit_helpers.isShowingModal(modal)).toBeFalsy();
expect(refreshed).toBeFalsy();
});
describe("Custom Buttons", function() {
var mockCustomButtonsHtml;
mockCustomButtonsHtml = readFixtures('mock/mock-xblock-editor-with-custom-buttons.underscore');
it('hides the modal\'s button bar', function() {
var requests = create_sinon.requests(this);
modal = showModal(requests, mockCustomButtonsHtml);
expect(modal.$('.modal-actions')).toBeHidden();
});
});
});
describe("XModule Editor", function() {
......@@ -64,12 +108,11 @@ define(["jquery", "underscore", "js/spec_helpers/create_sinon", "js/spec_helpers
mockXModuleEditorHtml = readFixtures('mock/mock-xmodule-editor.underscore');
beforeEach(function() {
// Mock the VerticalDescriptor so that the module can be rendered
window.VerticalDescriptor = XModule.Descriptor;
edit_helpers.installMockXModule();
});
afterEach(function () {
window.VerticalDescriptor = null;
edit_helpers.uninstallMockXModule();
});
it('can render itself', function() {
......@@ -140,12 +183,11 @@ define(["jquery", "underscore", "js/spec_helpers/create_sinon", "js/spec_helpers
mockXModuleEditorHtml = readFixtures('mock/mock-xmodule-settings-only-editor.underscore');
beforeEach(function() {
// Mock the VerticalDescriptor so that the module can be rendered
window.VerticalDescriptor = XModule.Descriptor;
edit_helpers.installMockXModule();
});
afterEach(function () {
window.VerticalDescriptor = null;
edit_helpers.uninstallMockXModule();
});
it('can render itself', function() {
......
cms/static/js/spec/views/pages/container_spec.js
......@@ -107,6 +107,29 @@ define(["jquery", "js/spec_helpers/create_sinon", "js/spec_helpers/edit_helpers"
});
expect(edit_helpers.isShowingModal()).toBeTruthy();
});
});
describe("Editing an xmodule", function() {
var mockContainerXBlockHtml,
mockXModuleEditor,
newDisplayName = 'New Display Name';
beforeEach(function () {
edit_helpers.installMockXModule({
data: "<p>Some HTML</p>",
metadata: {
display_name: newDisplayName
}
});
});
afterEach(function() {
edit_helpers.uninstallMockXModule();
edit_helpers.cancelModalIfShowing();
});
mockContainerXBlockHtml = readFixtures('mock/mock-container-xblock.underscore');
mockXModuleEditor = readFixtures('mock/mock-xmodule-editor.underscore');
it('can save changes to settings', function() {
var editButtons, modal, mockUpdatedXBlockHtml;
......@@ -117,7 +140,7 @@ define(["jquery", "js/spec_helpers/create_sinon", "js/spec_helpers/edit_helpers"
expect(editButtons.length).toBe(6);
editButtons.first().click();
create_sinon.respondWithJson(requests, {
html: mockXBlockEditorHtml,
html: mockXModuleEditor,
resources: []
});
......
cms/static/js/spec/views/xblock_editor_spec.js
......@@ -49,20 +49,6 @@ define([ "jquery", "underscore", "js/spec_helpers/create_sinon", "js/spec_helper
expect(editor.$el.select('.xblock-header')).toBeTruthy();
expect(editor.getMode()).toEqual('settings');
});
it('saves any custom metadata', function() {
var requests = create_sinon.requests(this), request, response;
editor.render();
create_sinon.respondWithJson(requests, {
html: mockXBlockEditorHtml,
resources: []
});
editor.save();
request = requests[requests.length - 1];
response = JSON.parse(request.requestBody);
expect(response.metadata.display_name).toBe(testDisplayName);
expect(response.metadata.custom_field).toBe('Custom Value');
});
});
describe("Editing an xmodule", function() {
......
cms/static/js/spec_helpers/edit_helpers.js
/**
* Provides helper methods for invoking Studio editors in Jasmine tests.
*/
define(["jquery", "js/spec_helpers/create_sinon", "js/spec_helpers/modal_helpers", "js/views/modals/edit_xblock",
"xmodule", "coffee/src/main", "xblock/cms.runtime.v1"],
function($, create_sinon, modal_helpers, EditXBlockModal) {
define(["jquery", "underscore", "js/spec_helpers/create_sinon", "js/spec_helpers/modal_helpers",
"js/views/modals/edit_xblock", "xmodule", "coffee/src/main", "xblock/cms.runtime.v1"],
function($, _, create_sinon, modal_helpers, EditXBlockModal) {
var editorTemplate = readFixtures('metadata-editor.underscore'),
numberEntryTemplate = readFixtures('metadata-number-entry.underscore'),
......@@ -12,19 +12,15 @@ define(["jquery", "js/spec_helpers/create_sinon", "js/spec_helpers/modal_helpers
editorModeButtonTemplate = readFixtures('editor-mode-button.underscore'),
installMockXBlock,
uninstallMockXBlock,
hasSavedMockXBlock,
installMockXModule,
uninstallMockXModule,
hasSavedMockXModule,
installEditTemplates,
showEditModal;
installMockXBlock = function(mockResult) {
window.MockXBlock = function(runtime, element) {
return {
save: function() {
return mockResult;
}
runtime: runtime
};
};
};
......@@ -58,9 +54,9 @@ define(["jquery", "js/spec_helpers/create_sinon", "js/spec_helpers/modal_helpers
appendSetFixtures($("<script>", {id: "metadata-string-entry", type: "text/template"}).text(stringEntryTemplate));
};
showEditModal = function(requests, xblockElement, model, mockHtml) {
showEditModal = function(requests, xblockElement, model, mockHtml, options) {
var modal = new EditXBlockModal({});
modal.edit(xblockElement, model);
modal.edit(xblockElement, model, options);
create_sinon.respondWithJson(requests, {
html: mockHtml,
"resources": []
......
cms/static/js/spec_helpers/modal_helpers.js
......@@ -47,7 +47,7 @@ define(["jquery"],
cancelModal = function(modal) {
var modalElement, cancelButton;
modalElement = getModalElement(modal);
cancelButton = modalElement.find('.action-cancel');
cancelButton = modalElement.find('.action-cancel:visible');
expect(cancelButton.length).toBe(1);
cancelButton.click();
};
......
cms/static/js/views/modals/base_modal.js
......@@ -71,8 +71,10 @@ define(["jquery", "underscore", "gettext", "js/views/baseview"],
},
cancel: function(event) {
if (event) {
event.preventDefault();
event.stopPropagation(); // Make sure parent modals don't see the click
}
this.hide();
},
......@@ -98,7 +100,21 @@ define(["jquery", "underscore", "gettext", "js/views/baseview"],
name: name,
isPrimary: isPrimary
});
this.$('.modal-actions ul').append(html);
this.getActionBar().find('ul').append(html);
},
/**
* Returns the action bar that contains the modal's action buttons.
*/
getActionBar: function() {
return this.$('.modal-window > div > .modal-actions');
},
/**
* Returns the action button of the specified type.
*/
getActionButton: function(type) {
return this.getActionBar().find('.action-' + type);
},
resize: function() {
......
cms/static/js/views/modals/edit_xblock.js
......@@ -37,6 +37,10 @@ define(["jquery", "underscore", "gettext", "js/views/modals/base_modal",
this.editOptions = options;
this.render();
this.show();
// Hide the action bar until we know which buttons we want
this.getActionBar().hide();
// Display the xblock after the modal is shown as there are some xblocks
// that depend upon being visible when they initialize, e.g. the problem xmodule.
this.displayXBlock();
......@@ -60,7 +64,17 @@ define(["jquery", "underscore", "gettext", "js/views/modals/base_modal",
onDisplayXBlock: function() {
var editorView = this.editorView,
title = this.getTitle();
title = this.getTitle(),
xblock = editorView.xblock,
runtime = xblock.runtime;
// Notify the runtime that the modal has been shown
if (runtime) {
this.runtime = runtime;
runtime.notify("edit-modal-shown", this);
}
// Update the modal's header
if (editorView.hasCustomTabs()) {
// Hide the modal's header as the custom editor provides its own
this.$('.modal-header').hide();
......@@ -74,9 +88,28 @@ define(["jquery", "underscore", "gettext", "js/views/modals/base_modal",
this.selectMode(editorView.mode);
}
}
// If the xblock is not using custom buttons then choose which buttons to show
if (!editorView.hasCustomButtons()) {
// If the xblock does not support save then disable the save button
if (!xblock.save) {
this.disableSave();
}
this.getActionBar().show();
}
// Resize the modal to fit the window
this.resize();
},
disableSave: function() {
var saveButton = this.getActionButton('save'),
cancelButton = this.getActionButton('cancel');
saveButton.hide();
cancelButton.text(gettext('OK'));
cancelButton.addClass('action-primary');
},
getTitle: function() {
var displayName = this.xblockElement.find('.xblock-header .header-details').text().trim();
// If not found, try the old unit page style rendering
......@@ -117,23 +150,28 @@ define(["jquery", "underscore", "gettext", "js/views/modals/base_modal",
},
save: function(event) {
var self = this,
xblockInfo = this.xblockInfo,
refresh = self.editOptions.refresh;
event.preventDefault();
this.editorView.save({
success: function() {
self.hide();
success: _.bind(this.onSave, this)
});
},
onSave: function() {
var refresh = this.editOptions.refresh;
this.hide();
if (refresh) {
refresh(xblockInfo);
refresh(this.xblockInfo);
}
}
});
},
hide: function() {
BaseModal.prototype.hide.call(this);
// Notify the runtime that the modal has been hidden
if (this.runtime) {
this.runtime.notify('edit-modal-hidden');
}
// Completely clear the contents of the modal
this.undelegateEvents();
this.$el.html("");
......
cms/static/js/views/xblock_editor.js
......@@ -49,6 +49,10 @@ define(["jquery", "underscore", "gettext", "js/views/feedback_notification", "js
return this.$('.editor-with-tabs').length > 0;
},
hasCustomButtons: function() {
return this.$('.editor-with-buttons').length > 0;
},
createMetadataEditor: function() {
var metadataEditor,
metadataData,
......@@ -88,7 +92,8 @@ define(["jquery", "underscore", "gettext", "js/views/feedback_notification", "js
var xblockInfo = this.model,
data,
saving;
data = this.getXBlockData();
data = this.getXModuleData();
if (data) {
saving = new NotificationView.Mini({
title: gettext('Saving&hellip;')
});
......@@ -100,16 +105,24 @@ define(["jquery", "underscore", "gettext", "js/views/feedback_notification", "js
success();
}
});
}
},
getXBlockData: function() {
/**
* Returns the data saved for the xmodule. Note that this *does not* work for XBlocks.
*/
getXModuleData: function() {
var xblock = this.xblock,
metadataEditor = this.getMetadataEditor(),
data;
data = null;
if (xblock.save) {
data = xblock.save();
if (metadataEditor) {
data.metadata = _.extend(data.metadata || {}, this.getChangedMetadata());
}
} else {
console.error('Cannot save xblock as it has no save method');
}
return data;
},
......
cms/static/sass/elements/_modal-window.scss
......@@ -18,7 +18,6 @@
.modal-content {
position: relative;
box-shadow: 0 0 3px $shadow-d1;
background-color: $white;
padding: 5%;
}
......@@ -52,7 +51,9 @@
}
}
// TODO: need to sync up (alongside general editing mode) with xblocks.scss UI
.modal-chin,
.xblock-actions,
.modal-actions {
padding: ($baseline*0.75) 2% ($baseline/2) 2%;
......@@ -190,6 +191,22 @@
}
}
// xblock custom actions
.modal-window .editor-with-buttons {
margin-bottom: ($baseline*3);
// TODO: need to sync up (alongside general editing mode) with xblocks.scss UI
.xblock-actions {
background-color: $gray-l4;
position: absolute;
width: 100%;
bottom: 0;
}
}
// special overrides for video module editor/hidden tab editors
.modal-lg.modal-type-video {
......
cms/templates/component.html
<%! from django.utils.translation import ugettext as _ %>
<%namespace name='static' file='static_content.html'/>
<div class="wrapper wrapper-component-editor">
<div class="component-editor">
<div class="component-edit-header">
<span class="component-name"></span>
<ul class="nav-edit-modes">
<li id="editor-mode" class="mode active-mode" aria-controls="editor-tab" role="tab">
<a href="#">${_("Editor")}</a>
</li>
<li id="settings-mode" class="mode active-mode" aria-controls="settings-tab" role="tab">
<a href="#">${_("Settings")}</a>
</li>
</ul>
</div> <!-- Editor Header -->
<div class="component-edit-modes">
<div class="module-editor"/>
</div>
## Native XBlocks render their own save/cancel buttons
% if show_save_cancel:
<div class="row module-actions">
<a href="#" class="save-button action-primary action">${_("Save")}</a>
<a href="#" class="cancel-button action-secondary action">${_("Cancel")}</a>
</div> <!-- Module Actions-->
% endif
</div>
</div>
<div class="wrapper wrapper-component-action-header">
<div class="component-header">
${label}
......
cms/templates/container.html
......@@ -130,7 +130,4 @@ main_xblock_info = {
</section>
</div>
</div>
<div class="edit-xblock-modal"></div>
</%block>
cms/templates/edit-tabs.html
......@@ -178,6 +178,4 @@
<span class="label">${_("close modal")}</span>
</a>
</div>
<div class="edit-xblock-modal"></div>
</%block>
cms/templates/js/basic-modal.underscore
......@@ -5,7 +5,7 @@
role="dialog">
<div class="modal-window-overlay"></div>
<div class="modal-window confirm modal-editor modal-<%= size %> modal-type-<%= type %>">
<div class="<%= name %>-modal" action="#">
<div class="<%= name %>-modal">
<div class="modal-header">
<h2 class="title modal-window-title"><%= title %></h2>
<ul class="editor-modes action-list action-modes">
......
cms/templates/js/mock/mock-xblock-editor-with-custom-buttons.underscore 0 → 100644
<div class="xblock xblock-studio_view" data-runtime-version="1" data-usage-id="i4x:;_;_edX;_mock"
data-init="MockXBlock" data-runtime-class="StudioRuntime" data-block-type="mock" tabindex="0">
<div class="mock-xblock editor-with-buttons">
<h3>Mock XBlock Editor</h3>
<div class="modal-actions">
<h3 class="sr">Actions</h3>
<ul>
<li class="action-item">
<a href="#" class="button action-primary action-save">Save</a>
</li>
<li class="action-item">
<a href="#" class="button action-cancel">Cancel</a>
</li>
</ul>
</div>
</div>
</div>
cms/templates/unit.html
......@@ -225,5 +225,4 @@ require(["domReady!", "jquery", "js/models/module_info", "coffee/src/views/unit"
</div>
</div>
</div>
<div class="edit-xblock-modal"></div>
</%block>
common/static/coffee/src/xblock/core.coffee
......@@ -12,6 +12,7 @@
runtime = new window[runtime]["v#{version}"]
initFn = window[initFnName]
block = initFn(runtime, element) ? {}
block.runtime = runtime
else
elementTag = $('<div>').append($element.clone()).html();
console.log("Block #{elementTag} is missing data-runtime, data-runtime-version or data-init, and can't be initialized")
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment