From 24e58373899444468223fccf86e719066f050598 Mon Sep 17 00:00:00 2001 From: Daniel Friedman Date: Tue, 8 Jul 2014 12:55:23 -0400 Subject: [PATCH] Fix whitespace problems in xblock string field editor. Whitespace is now trimmed from input. If after trimming input is empty (pure whitespace or the empty string), the field is not updated. --- .../js/spec/views/pages/container_spec.js | 51 +++++++++++++++---- .../js/views/xblock_string_field_editor.js | 18 +++++-- 2 files changed, 54 insertions(+), 15 deletions(-) diff --git a/cms/static/js/spec/views/pages/container_spec.js b/cms/static/js/spec/views/pages/container_spec.js index b053a3da9d..f9fcd5d8a8 100644 --- a/cms/static/js/spec/views/pages/container_spec.js +++ b/cms/static/js/spec/views/pages/container_spec.js @@ -86,7 +86,7 @@ define(["jquery", "underscore", "underscore.string", "js/spec_helpers/create_sin describe("Editing the container", function() { var updatedDisplayName = 'Updated Test Container', - inlineEditDisplayName, displayNameElement, displayNameInput; + expectEditCanceled, inlineEditDisplayName, displayNameElement, displayNameInput; beforeEach(function() { displayNameElement = containerPage.$('.page-header-title'); @@ -104,6 +104,25 @@ define(["jquery", "underscore", "underscore.string", "js/spec_helpers/create_sin displayNameInput.val(newTitle); }; + expectEditCanceled = function(options) { + var initialRequests; + renderContainerPage(mockContainerXBlockHtml, options.that); + initialRequests = requests.length; + inlineEditDisplayName(options.newTitle); + if (options.pressEscape) { + displayNameInput.simulate("keydown", { keyCode: $.simulate.keyCode.ESCAPE }); + displayNameInput.simulate("keyup", { keyCode: $.simulate.keyCode.ESCAPE }); + } else { + displayNameInput.change(); + } + // No requests should be made when the edit is cancelled client-side + expect(initialRequests).toBe(requests.length); + expect(displayNameInput).toHaveClass('is-hidden'); + expect(displayNameElement).not.toHaveClass('is-hidden'); + expect(displayNameInput.val()).toBe(initialDisplayName); + expect(containerPage.model.get('display_name')).toBe(initialDisplayName); + }; + it('can edit itself', function() { var editButtons; renderContainerPage(mockContainerXBlockHtml, this); @@ -167,18 +186,30 @@ define(["jquery", "underscore", "underscore.string", "js/spec_helpers/create_sin expect(containerPage.model.get('display_name')).toBe(initialDisplayName); }); - it('can cancel an inline edit', function() { - var numRequests; + it('trims whitespace from the display name', function() { renderContainerPage(mockContainerXBlockHtml, this); - inlineEditDisplayName(updatedDisplayName); - numRequests = requests.length; - displayNameInput.simulate("keydown", { keyCode: $.simulate.keyCode.ESCAPE }); - displayNameInput.simulate("keyup", { keyCode: $.simulate.keyCode.ESCAPE }); - expect(requests.length).toBe(numRequests); + inlineEditDisplayName(updatedDisplayName + ' '); + displayNameInput.change(); + // This is the response for the change operation. + create_sinon.respondWithJson(requests, { }); + // This is the response for the subsequent fetch operation. + create_sinon.respondWithJson(requests, {"display_name": updatedDisplayName}); expect(displayNameInput).toHaveClass('is-hidden'); expect(displayNameElement).not.toHaveClass('is-hidden'); - expect(displayNameElement.text().trim()).toBe(initialDisplayName); - expect(containerPage.model.get('display_name')).toBe(initialDisplayName); + expect(displayNameElement.text()).toBe(updatedDisplayName); + expect(containerPage.model.get('display_name')).toBe(updatedDisplayName); + }); + + it('does not change the title when input is the empty string', function() { + expectEditCanceled({newTitle: '', pressEscape: false, that: this}); + }); + + it('does not change the title when input is whitespace-only', function() { + expectEditCanceled({newTitle: ' ', pressEscape: false, that: this}); + }); + + it('can cancel an inline edit', function() { + expectEditCanceled({newTitle: updatedDisplayName, pressEscape: true, that: this}); }); }); diff --git a/cms/static/js/views/xblock_string_field_editor.js b/cms/static/js/views/xblock_string_field_editor.js index a4b44e0664..943c389cc3 100644 --- a/cms/static/js/views/xblock_string_field_editor.js +++ b/cms/static/js/views/xblock_string_field_editor.js @@ -68,11 +68,20 @@ define(["jquery", "gettext", "js/views/baseview"], this.getInput().addClass('is-hidden'); }, + cancelInput: function() { + this.getInput().val(this.model.get(this.fieldName)); + this.hideInput(); + }, + updateField: function() { var xblockInfo = this.model, - newValue = this.getInput().val(), - requestData = this.createUpdateRequestData(newValue), - fieldName = this.fieldName; + newValue = this.getInput().val().trim(), + oldValue = xblockInfo.get(this.fieldName), + requestData = this.createUpdateRequestData(newValue); + if (newValue === '' || newValue === oldValue) { + this.cancelInput(); + return; + } this.runOperationShowingMessage(gettext('Saving…'), function() { return xblockInfo.save(requestData); @@ -92,8 +101,7 @@ define(["jquery", "gettext", "js/views/baseview"], handleKeyUp: function(event) { if (event.keyCode === 27) { // Revert the changes if the user hits escape - this.getInput().val(this.model.get(this.fieldName)); - this.hideInput(); + this.cancelInput(); } } });