From cf70eb6e2ad5e56ade7e44c38d4211dc99243fe1 Mon Sep 17 00:00:00 2001 From: Frances Botsford Date: Thu, 10 Jul 2014 21:24:59 -0400 Subject: [PATCH] in-context editor for xblock string field --- .../js/spec/views/pages/container_spec.js | 38 ++++++++++-------- .../spec/views/pages/course_outline_spec.js | 30 +++++++++----- cms/static/js/spec_helpers/view_helpers.js | 23 +++++------ cms/static/js/views/pages/container.js | 7 ---- .../js/views/xblock_string_field_editor.js | 18 ++++++--- cms/static/sass/elements/_forms.scss | 33 ++++++++++++++++ cms/static/sass/views/_container.scss | 39 +++++++++---------- cms/templates/container.html | 5 ++- cms/templates/js/course-outline.underscore | 4 +- .../js/mock/mock-container-page.underscore | 4 +- cms/templates/js/xblock-outline.underscore | 4 +- .../js/xblock-string-field-editor.underscore | 16 ++++++-- common/static/sass/_mixins.scss | 20 ++++++++-- .../test/acceptance/pages/studio/overview.py | 3 +- 14 files changed, 155 insertions(+), 89 deletions(-) diff --git a/cms/static/js/spec/views/pages/container_spec.js b/cms/static/js/spec/views/pages/container_spec.js index 08b0768367..a6e5a3c02c 100644 --- a/cms/static/js/spec/views/pages/container_spec.js +++ b/cms/static/js/spec/views/pages/container_spec.js @@ -95,18 +95,22 @@ define(["jquery", "underscore", "underscore.string", "js/spec_helpers/create_sin describe("Editing the container", function() { var updatedDisplayName = 'Updated Test Container', - expectEditCanceled; + expectEditCanceled, getDisplayNameWrapper; afterEach(function() { edit_helpers.cancelModalIfShowing(); }); + getDisplayNameWrapper = function() { + return containerPage.$('.wrapper-xblock-field'); + }; + expectEditCanceled = function(test, options) { - var initialRequests, displayNameElement, displayNameInput; + var initialRequests, displayNameWrapper; renderContainerPage(mockContainerXBlockHtml, test); + displayNameWrapper = getDisplayNameWrapper(); initialRequests = requests.length; - displayNameElement = containerPage.$('.page-header-title'); - displayNameInput = edit_helpers.inlineEdit(displayNameElement, options.newTitle); + displayNameInput = edit_helpers.inlineEdit(displayNameWrapper, options.newTitle); if (options.pressEscape) { displayNameInput.simulate("keydown", { keyCode: $.simulate.keyCode.ESCAPE }); displayNameInput.simulate("keyup", { keyCode: $.simulate.keyCode.ESCAPE }); @@ -115,7 +119,7 @@ define(["jquery", "underscore", "underscore.string", "js/spec_helpers/create_sin } // No requests should be made when the edit is cancelled client-side expect(initialRequests).toBe(requests.length); - edit_helpers.verifyInlineEditChange(displayNameElement, initialDisplayName); + edit_helpers.verifyInlineEditChange(displayNameWrapper, initialDisplayName); expect(containerPage.model.get('display_name')).toBe(initialDisplayName); }; @@ -157,44 +161,44 @@ define(["jquery", "underscore", "underscore.string", "js/spec_helpers/create_sin }); it('can inline edit the display name', function() { - var displayNameElement, displayNameInput; + var displayNameInput, displayNameWrapper; renderContainerPage(mockContainerXBlockHtml, this); - displayNameElement = containerPage.$('.page-header-title'); - displayNameInput = edit_helpers.inlineEdit(displayNameElement, updatedDisplayName); + displayNameWrapper = getDisplayNameWrapper(); + displayNameInput = edit_helpers.inlineEdit(displayNameWrapper, 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}); - edit_helpers.verifyInlineEditChange(displayNameElement, updatedDisplayName); + edit_helpers.verifyInlineEditChange(displayNameWrapper, updatedDisplayName); expect(containerPage.model.get('display_name')).toBe(updatedDisplayName); }); it('does not change the title when a display name update fails', function() { - var initialRequests, displayNameElement, displayNameInput; + var initialRequests, displayNameInput, displayNameWrapper; renderContainerPage(mockContainerXBlockHtml, this); - displayNameElement = containerPage.$('.page-header-title'); - displayNameInput = edit_helpers.inlineEdit(displayNameElement, updatedDisplayName); + displayNameWrapper = getDisplayNameWrapper(); + displayNameInput = edit_helpers.inlineEdit(displayNameWrapper, updatedDisplayName); initialRequests = requests.length; displayNameInput.change(); create_sinon.respondWithError(requests); // No fetch operation should occur. expect(initialRequests + 1).toBe(requests.length); - edit_helpers.verifyInlineEditChange(displayNameElement, initialDisplayName, updatedDisplayName); + edit_helpers.verifyInlineEditChange(displayNameWrapper, initialDisplayName, updatedDisplayName); expect(containerPage.model.get('display_name')).toBe(initialDisplayName); }); it('trims whitespace from the display name', function() { - var displayNameElement, displayNameInput; + var displayNameInput, displayNameWrapper; renderContainerPage(mockContainerXBlockHtml, this); - displayNameElement = containerPage.$('.page-header-title'); - displayNameInput = edit_helpers.inlineEdit(displayNameElement, updatedDisplayName + ' '); + displayNameWrapper = getDisplayNameWrapper(); + displayNameInput = edit_helpers.inlineEdit(displayNameWrapper, 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}); - edit_helpers.verifyInlineEditChange(displayNameElement, updatedDisplayName); + edit_helpers.verifyInlineEditChange(displayNameWrapper, updatedDisplayName); expect(containerPage.model.get('display_name')).toBe(updatedDisplayName); }); diff --git a/cms/static/js/spec/views/pages/course_outline_spec.js b/cms/static/js/spec/views/pages/course_outline_spec.js index c78a14df38..727b183dc1 100644 --- a/cms/static/js/spec/views/pages/course_outline_spec.js +++ b/cms/static/js/spec/views/pages/course_outline_spec.js @@ -244,6 +244,12 @@ define(["jquery", "js/spec_helpers/create_sinon", "js/spec_helpers/view_helpers" }); describe("Section", function() { + var getDisplayNameWrapper; + + getDisplayNameWrapper = function() { + return getHeaderElement('.outline-item-section').find('.wrapper-xblock-field').first(); + }; + it('can be deleted', function() { var promptSpy = view_helpers.createPromptSpy(), requestCount; createCourseOutlinePage(this, createMockCourseJSON('mock-course', 'Mock Course', [ @@ -306,17 +312,17 @@ define(["jquery", "js/spec_helpers/create_sinon", "js/spec_helpers/view_helpers" it('can be renamed inline', function() { var updatedDisplayName = 'Updated Section Name', - displayNameElement, + displayNameWrapper, sectionModel; createCourseOutlinePage(this, mockCourseJSON); - displayNameElement = getHeaderElement('.outline-item-section').find('.xblock-field-value'); - displayNameInput = view_helpers.inlineEdit(displayNameElement, updatedDisplayName); + displayNameWrapper = getDisplayNameWrapper(); + displayNameInput = view_helpers.inlineEdit(displayNameWrapper, 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}); - view_helpers.verifyInlineEditChange(displayNameElement, updatedDisplayName); + view_helpers.verifyInlineEditChange(displayNameWrapper, updatedDisplayName); sectionModel = outlinePage.model.get('child_info').children[0]; expect(sectionModel.get('display_name')).toBe(updatedDisplayName); }); @@ -330,6 +336,12 @@ define(["jquery", "js/spec_helpers/create_sinon", "js/spec_helpers/view_helpers" }); describe("Subsection", function() { + var getDisplayNameWrapper; + + getDisplayNameWrapper = function() { + return getHeaderElement('.outline-item-subsection').find('.wrapper-xblock-field').first(); + }; + it('can be deleted', function() { var promptSpy = view_helpers.createPromptSpy(); createCourseOutlinePage(this, mockCourseJSON); @@ -361,11 +373,11 @@ define(["jquery", "js/spec_helpers/create_sinon", "js/spec_helpers/view_helpers" it('can be renamed inline', function() { var updatedDisplayName = 'Updated Subsection Name', - displayNameElement, + displayNameWrapper, subsectionModel; createCourseOutlinePage(this, mockCourseJSON); - displayNameElement = getHeaderElement('.outline-item-subsection').find('.xblock-field-value'); - displayNameInput = view_helpers.inlineEdit(displayNameElement, updatedDisplayName); + displayNameWrapper = getDisplayNameWrapper(); + displayNameInput = view_helpers.inlineEdit(displayNameWrapper, updatedDisplayName); displayNameInput.change(); // This is the response for the change operation. create_sinon.respondWithJson(requests, { }); @@ -375,8 +387,8 @@ define(["jquery", "js/spec_helpers/create_sinon", "js/spec_helpers/view_helpers" createMockSubsectionJSON('mock-subsection', updatedDisplayName, []) ])); // Find the display name again in the refreshed DOM and verify it - displayNameElement = getHeaderElement('.outline-item-subsection').find('.xblock-field-value'); - view_helpers.verifyInlineEditChange(displayNameElement, updatedDisplayName); + displayNameWrapper = getHeaderElement('.outline-item-subsection').find('.wrapper-xblock-field').first(); + view_helpers.verifyInlineEditChange(displayNameWrapper, updatedDisplayName); subsectionModel = outlinePage.model.get('child_info').children[0].get('child_info').children[0]; expect(subsectionModel.get('display_name')).toBe(updatedDisplayName); }); diff --git a/cms/static/js/spec_helpers/view_helpers.js b/cms/static/js/spec_helpers/view_helpers.js index f856ddbfa2..6eeea44ac0 100644 --- a/cms/static/js/spec_helpers/view_helpers.js +++ b/cms/static/js/spec_helpers/view_helpers.js @@ -101,25 +101,22 @@ define(["jquery", "js/views/feedback_notification", "js/views/feedback_prompt"], delete window.course_location_analytics; }; - inlineEdit = function(element, newValue) { - var inputField; - element.click(); - expect(element).toHaveClass('is-hidden'); - inputField = element.next().find('.xblock-field-input'); - expect(inputField).not.toHaveClass('is-hidden'); + inlineEdit = function(editorWrapper, newValue) { + var inputField = editorWrapper.find('.xblock-field-input'), + editButton = editorWrapper.find('.xblock-field-value-edit'); + editButton.click(); + expect(editorWrapper).toHaveClass('is-editing'); inputField.val(newValue); return inputField; }; - verifyInlineEditChange = function(element, expectedValue, failedValue) { - var inputField = element.next().find('.xblock-field-input'); - expect(element.text()).toBe(expectedValue); + verifyInlineEditChange = function(editorWrapper, expectedValue, failedValue) { + var displayName = editorWrapper.find('.xblock-field-value'); + expect(displayName.text()).toBe(expectedValue); if (failedValue) { - expect(element).toHaveClass('is-hidden'); - expect(inputField).not.toHaveClass('is-hidden'); + expect(editorWrapper).toHaveClass('is-editing'); } else { - expect(element).not.toHaveClass('is-hidden'); - expect(inputField).toHaveClass('is-hidden'); + expect(editorWrapper).not.toHaveClass('is-editing'); } }; diff --git a/cms/static/js/views/pages/container.js b/cms/static/js/views/pages/container.js index c5cd824e3e..5d43ff6ed0 100644 --- a/cms/static/js/views/pages/container.js +++ b/cms/static/js/views/pages/container.js @@ -24,7 +24,6 @@ define(["jquery", "underscore", "gettext", "js/views/pages/base_page", "js/views if (this.options.action === 'new') { this.nameEditor.$('.xblock-field-value').click(); } - this.model.on('sync', this.onSync, this); this.xblockView = new ContainerView({ el: this.$('.wrapper-xblock'), model: this.model, @@ -60,12 +59,6 @@ define(["jquery", "underscore", "gettext", "js/views/pages/base_page", "js/views } }, - onSync: function(model) { - if (ViewUtils.hasChangedAttributes(model, ['display_name'])) { - this.render(); - } - }, - render: function(options) { var self = this, xblockView = this.xblockView, diff --git a/cms/static/js/views/xblock_string_field_editor.js b/cms/static/js/views/xblock_string_field_editor.js index 1cb2bac6a5..033fdd5497 100644 --- a/cms/static/js/views/xblock_string_field_editor.js +++ b/cms/static/js/views/xblock_string_field_editor.js @@ -10,7 +10,8 @@ define(["js/views/baseview", "js/views/utils/xblock_utils"], var XBlockStringFieldEditor = BaseView.extend({ events: { - 'click .xblock-field-value': 'showInput', + 'click .xblock-field-value-edit': 'showInput', + 'click button[type=submit]': 'onClickSubmit', 'change .xblock-field-input': 'updateField', 'focusout .xblock-field-input': 'onInputFocusLost', 'keyup .xblock-field-input': 'handleKeyUp' @@ -21,6 +22,7 @@ define(["js/views/baseview", "js/views/utils/xblock_utils"], initialize: function() { BaseView.prototype.initialize.call(this); this.fieldName = this.$el.data('field'); + this.fieldDisplayName = this.$el.data('field-display-name'); this.template = this.loadTemplate('xblock-string-field-editor'); this.model.on('change:' + this.fieldName, this.onChangeField, this); }, @@ -28,7 +30,8 @@ define(["js/views/baseview", "js/views/utils/xblock_utils"], render: function() { this.$el.append(this.template({ value: this.model.get(this.fieldName), - fieldName: this.fieldName + fieldName: this.fieldName, + fieldDisplayName: this.fieldDisplayName })); return this; }, @@ -48,6 +51,11 @@ define(["js/views/baseview", "js/views/utils/xblock_utils"], } }, + onClickSubmit: function(event) { + event.preventDefault(); + this.updateField(); + }, + onChangeField: function() { var value = this.model.get(this.fieldName); this.getLabel().text(value); @@ -58,14 +66,12 @@ define(["js/views/baseview", "js/views/utils/xblock_utils"], showInput: function(event) { var input = this.getInput(); event.preventDefault(); - this.getLabel().addClass('is-hidden'); - input.removeClass('is-hidden'); + this.$el.addClass('is-editing'); input.focus(); }, hideInput: function() { - this.getLabel().removeClass('is-hidden'); - this.getInput().addClass('is-hidden'); + this.$el.removeClass('is-editing'); }, cancelInput: function() { diff --git a/cms/static/sass/elements/_forms.scss b/cms/static/sass/elements/_forms.scss index d1b228b8ea..1c1ff8eef0 100644 --- a/cms/static/sass/elements/_forms.scss +++ b/cms/static/sass/elements/_forms.scss @@ -326,6 +326,39 @@ form[class^="create-"] { } } + +// form - inline xblock name edit on unit, container, outline? + +.incontext-editor.is-editable { + + .incontext-editor-value, + .incontext-editor-action-wrapper { + display: inline-block; + } + + .incontext-editor-open-action { + @extend %ui-btn-non-blue; + @extend %t-copy-base; + padding-top: ($baseline/10); + } + + .incontext-editor-form { + display: none; + } + + &.is-editing { + + .incontext-editor-value, + .incontext-editor-action-wrapper { + display: none; + } + + .incontext-editor-form { + display: inline-block; + } + } +} + // ==================== // forms - grandfathered diff --git a/cms/static/sass/views/_container.scss b/cms/static/sass/views/_container.scss index 92f0fe4489..2a015c3f23 100644 --- a/cms/static/sass/views/_container.scss +++ b/cms/static/sass/views/_container.scss @@ -16,19 +16,25 @@ border-bottom: none; padding-bottom: 0; - .page-header-title { - @extend %t-title; - @include font-size(28); - @include line-height(32); - font-weight: 600; - } + .page-header { - .xblock-title .xblock-field-input { - @extend %t-title4; - background: none repeat scroll 0 0 white; - border: 0; - box-shadow: 0 0 2px 2px $shadow inset; - font-weight: 600; + .page-header-title { + @extend %t-title; + @include font-size(28); + @include line-height(32); + font-weight: 600; + } + + .is-editable { + + .incontext-editor-input { + @extend %t-title4; + background: none repeat scroll 0 0 white; + border: 0; + box-shadow: 0 0 2px 2px $shadow inset; + font-weight: 600; + } + } } &.has-actions { @@ -49,19 +55,12 @@ .no-container-content { @extend %no-content; + padding: ($baseline*1.5) ($baseline*2); // custom rules to reuse xblock validation styling in ui-well context .icon-warning-sign { display: none; } - - .edit-button { - @include green-button; - @extend %t-action4; - padding: 8px 20px 10px; - text-align: center; - margin: ($baseline/2) 0 ($baseline/2) $baseline; - } } .container-message { diff --git a/cms/templates/container.html b/cms/templates/container.html index e9450e5145..6cd6391a13 100644 --- a/cms/templates/container.html +++ b/cms/templates/container.html @@ -78,8 +78,9 @@ templates = ["basic-modal", "modal-button", "edit-xblock-modal", % endif % endfor -
-

${xblock.display_name_with_default | h}

+
+

${xblock.display_name_with_default | h}

diff --git a/cms/templates/js/course-outline.underscore b/cms/templates/js/course-outline.underscore index fd10856d0c..1034cf2860 100644 --- a/cms/templates/js/course-outline.underscore +++ b/cms/templates/js/course-outline.underscore @@ -14,8 +14,8 @@ <% if (xblockInfo.get('category') === 'vertical') { %> <%= xblockInfo.get('display_name') %> <% } else { %> - - <%= xblockInfo.get('display_name') %> + "> + <%= xblockInfo.get('display_name') %> <% } %> diff --git a/cms/templates/js/mock/mock-container-page.underscore b/cms/templates/js/mock/mock-container-page.underscore index b7d199178e..87a36330a8 100644 --- a/cms/templates/js/mock/mock-container-page.underscore +++ b/cms/templates/js/mock/mock-container-page.underscore @@ -6,8 +6,8 @@ Unit 1 -
-

Test Container

+
+

Test Container

diff --git a/cms/templates/js/xblock-outline.underscore b/cms/templates/js/xblock-outline.underscore index 0d7e02fdc6..2dec21afd6 100644 --- a/cms/templates/js/xblock-outline.underscore +++ b/cms/templates/js/xblock-outline.underscore @@ -15,8 +15,8 @@ <% if (xblockInfo.get('studio_url') && xblockInfo.get('category') !== 'chapter') { %> <%= xblockInfo.get('display_name') %> <% } else { %> - - <%= xblockInfo.get('display_name') %> + + <%= xblockInfo.get('display_name') %> <% } %> diff --git a/cms/templates/js/xblock-string-field-editor.underscore b/cms/templates/js/xblock-string-field-editor.underscore index 9eb7a41f6d..731b9df046 100644 --- a/cms/templates/js/xblock-string-field-editor.underscore +++ b/cms/templates/js/xblock-string-field-editor.underscore @@ -1,4 +1,12 @@ - - - +
<%= gettext("Edit") %>
+ +
+
+ <% var formLabel = gettext("Edit %(display_name)s (required)"); %> + + + +
+
diff --git a/common/static/sass/_mixins.scss b/common/static/sass/_mixins.scss index ba9486693c..2f24de41a3 100644 --- a/common/static/sass/_mixins.scss +++ b/common/static/sass/_mixins.scss @@ -247,11 +247,13 @@ // button with no button shell until hover for understated actions %ui-btn-non { @include transition(all .15s); - border: none; - border-radius: ($baseline/4); - background: none; - padding: 3px ($baseline/2); + @extend %ui-btn-pill; + @include transition(all $tmg-f2 linear 0s); + display: inline-block; vertical-align: middle; + border: none; + padding: 3px ($baseline/2); + background: none; color: $gray-l1; &:hover, &:focus { @@ -264,6 +266,16 @@ } } +// button with no button shell until hover for understated actions +%ui-btn-non-blue { + @extend %ui-btn-non; + + &:hover, &:focus { + background-color: $blue; + color: $white; + } +} + // extends - UI archetypes - well %ui-well { box-shadow: inset 0 1px 2px 1px $shadow; diff --git a/common/test/acceptance/pages/studio/overview.py b/common/test/acceptance/pages/studio/overview.py index 1e69ff8b84..ff3147bea4 100644 --- a/common/test/acceptance/pages/studio/overview.py +++ b/common/test/acceptance/pages/studio/overview.py @@ -14,6 +14,7 @@ class CourseOutlineItem(object): A mixin class for any :class:`PageObject` shown in a course outline. """ BODY_SELECTOR = None + EDIT_BUTTON_SELECTOR = '.xblock-title .xblock-field-value-edit' NAME_SELECTOR = '.xblock-title .xblock-field-value' NAME_INPUT_SELECTOR = '.xblock-title .xblock-field-input' @@ -45,7 +46,7 @@ class CourseOutlineItem(object): """ Changes the container's name. """ - self.q(css=self._bounded_selector(self.NAME_SELECTOR)).first.click() + self.q(css=self._bounded_selector(self.EDIT_BUTTON_SELECTOR)).first.click() set_input_value_and_save(self, self._bounded_selector(self.NAME_INPUT_SELECTOR), new_name) self.wait_for_ajax()