From eabfba48a20a9cc33ecc829bd1cb2dc9ef28b6ad Mon Sep 17 00:00:00 2001 From: Mushtaq Ali Date: Tue, 20 Jun 2017 17:05:23 +0500 Subject: [PATCH] Change video image requirements text hover - EDUCATOR-577 --- .../contentstore/views/tests/test_videos.py | 16 +++-- cms/djangoapps/contentstore/views/videos.py | 5 +- .../js/spec/views/video_thumbnail_spec.js | 12 ++-- cms/static/js/views/video_thumbnail.js | 71 ++++++++++++++----- cms/static/sass/views/_video-upload.scss | 56 ++++++++------- .../js/video-thumbnail-error.underscore | 8 +-- cms/templates/js/video-thumbnail.underscore | 6 +- requirements/edx/github.txt | 2 +- 8 files changed, 114 insertions(+), 62 deletions(-) diff --git a/cms/djangoapps/contentstore/views/tests/test_videos.py b/cms/djangoapps/contentstore/views/tests/test_videos.py index 96b554212e..f3f8a68cd2 100644 --- a/cms/djangoapps/contentstore/views/tests/test_videos.py +++ b/cms/djangoapps/contentstore/views/tests/test_videos.py @@ -717,7 +717,9 @@ class VideoImageTestCase(VideoUploadTestBase, CourseTestCase): 'width': 16, # 16x9 'height': 9 }, - 'The minimum allowed image resolution is {image_file_min_width}x{image_file_min_height}.'.format( + 'Recommended image resolution is {image_file_max_width}x{image_file_max_height}. The minimum resolution is {image_file_min_width}x{image_file_min_height}.'.format( + image_file_max_width=settings.VIDEO_IMAGE_MAX_WIDTH, + image_file_max_height=settings.VIDEO_IMAGE_MAX_HEIGHT, image_file_min_width=settings.VIDEO_IMAGE_MIN_WIDTH, image_file_min_height=settings.VIDEO_IMAGE_MIN_HEIGHT ) @@ -727,7 +729,9 @@ class VideoImageTestCase(VideoUploadTestBase, CourseTestCase): 'width': settings.VIDEO_IMAGE_MIN_WIDTH - 10, 'height': settings.VIDEO_IMAGE_MIN_HEIGHT }, - 'The minimum allowed image resolution is {image_file_min_width}x{image_file_min_height}.'.format( + 'Recommended image resolution is {image_file_max_width}x{image_file_max_height}. The minimum resolution is {image_file_min_width}x{image_file_min_height}.'.format( + image_file_max_width=settings.VIDEO_IMAGE_MAX_WIDTH, + image_file_max_height=settings.VIDEO_IMAGE_MAX_HEIGHT, image_file_min_width=settings.VIDEO_IMAGE_MIN_WIDTH, image_file_min_height=settings.VIDEO_IMAGE_MIN_HEIGHT ) @@ -737,7 +741,9 @@ class VideoImageTestCase(VideoUploadTestBase, CourseTestCase): 'width': settings.VIDEO_IMAGE_MIN_WIDTH, 'height': settings.VIDEO_IMAGE_MIN_HEIGHT - 10 }, - 'The minimum allowed image resolution is {image_file_min_width}x{image_file_min_height}.'.format( + 'Recommended image resolution is {image_file_max_width}x{image_file_max_height}. The minimum resolution is {image_file_min_width}x{image_file_min_height}.'.format( + image_file_max_width=settings.VIDEO_IMAGE_MAX_WIDTH, + image_file_max_height=settings.VIDEO_IMAGE_MAX_HEIGHT, image_file_min_width=settings.VIDEO_IMAGE_MIN_WIDTH, image_file_min_height=settings.VIDEO_IMAGE_MIN_HEIGHT ) @@ -747,7 +753,9 @@ class VideoImageTestCase(VideoUploadTestBase, CourseTestCase): 'width': 1200, # not 16:9, but width/height check first. 'height': 100 }, - 'The minimum allowed image resolution is {image_file_min_width}x{image_file_min_height}.'.format( + 'Recommended image resolution is {image_file_max_width}x{image_file_max_height}. The minimum resolution is {image_file_min_width}x{image_file_min_height}.'.format( + image_file_max_width=settings.VIDEO_IMAGE_MAX_WIDTH, + image_file_max_height=settings.VIDEO_IMAGE_MAX_HEIGHT, image_file_min_width=settings.VIDEO_IMAGE_MIN_WIDTH, image_file_min_height=settings.VIDEO_IMAGE_MIN_HEIGHT ) diff --git a/cms/djangoapps/contentstore/views/videos.py b/cms/djangoapps/contentstore/views/videos.py index 0226527d9f..c7b4fdd1f2 100644 --- a/cms/djangoapps/contentstore/views/videos.py +++ b/cms/djangoapps/contentstore/views/videos.py @@ -187,7 +187,10 @@ def validate_video_image(image_file): return _('This image file is corrupted.') image_file_aspect_ratio = abs(image_file_width / float(image_file_height) - settings.VIDEO_IMAGE_ASPECT_RATIO) if image_file_width < settings.VIDEO_IMAGE_MIN_WIDTH or image_file_height < settings.VIDEO_IMAGE_MIN_HEIGHT: - error = _('The minimum allowed image resolution is {image_file_min_width}x{image_file_min_height}.').format( + error = _('Recommended image resolution is {image_file_max_width}x{image_file_max_height}. ' + 'The minimum resolution is {image_file_min_width}x{image_file_min_height}.').format( + image_file_max_width=settings.VIDEO_IMAGE_MAX_WIDTH, + image_file_max_height=settings.VIDEO_IMAGE_MAX_HEIGHT, image_file_min_width=settings.VIDEO_IMAGE_MIN_WIDTH, image_file_min_height=settings.VIDEO_IMAGE_MIN_HEIGHT ) diff --git a/cms/static/js/spec/views/video_thumbnail_spec.js b/cms/static/js/spec/views/video_thumbnail_spec.js index 4cdb8b086c..ff903b13d5 100644 --- a/cms/static/js/spec/views/video_thumbnail_spec.js +++ b/cms/static/js/spec/views/video_thumbnail_spec.js @@ -201,12 +201,12 @@ define( AjaxHelpers.respondWithError(requests, 400); - verifyStateInfo($thumbnail, 'error'); + verifyStateInfo($thumbnail, 'edit'); }); it('calls readMessage with correct message', function() { - var errorMessage = 'This image file type is not supported. Supported file types are ' + - videoThumbnailView.getVideoImageSupportedFileFormats().humanize + '.', + var errorMessage = 'Image upload failed. This image file type is not supported. Supported file ' + + 'types are ' + videoThumbnailView.getVideoImageSupportedFileFormats().humanize + '.', successData = { files: [createFakeImageFile()], submit: function() {} @@ -255,7 +255,7 @@ define( // Verify error message expect($videoListEl.find('.thumbnail-error-wrapper').find('.action-text').html() .trim()).toEqual( - 'The selected image must be larger than ' + + 'Image upload failed. The selected image must be larger than ' + videoThumbnailView.getVideoImageMinSize().humanize + '.' ); }); @@ -270,7 +270,7 @@ define( // Verify error message expect($videoListEl.find('.thumbnail-error-wrapper').find('.action-text').html() .trim()).toEqual( - 'The selected image must be smaller than ' + + 'Image upload failed. The selected image must be smaller than ' + videoThumbnailView.getVideoImageMaxSize().humanize + '.' ); }); @@ -307,7 +307,7 @@ define( // Verify error message expect($videoListEl.find('.thumbnail-error-wrapper').find('.action-text').html() .trim()).toEqual( - 'This image file type is not supported. Supported file types are ' + + 'Image upload failed. This image file type is not supported. Supported file types are ' + videoThumbnailView.getVideoImageSupportedFileFormats().humanize + '.' ); }); diff --git a/cms/static/js/views/video_thumbnail.js b/cms/static/js/views/video_thumbnail.js index ebab2c4344..bd3180891d 100644 --- a/cms/static/js/views/video_thumbnail.js +++ b/cms/static/js/views/video_thumbnail.js @@ -26,26 +26,42 @@ define( this.videoImageSettings = options.videoImageSettings; this.actionsInfo = { upload: { + name: 'upload', icon: '', text: gettext('Add Thumbnail') }, edit: { + name: 'edit', + actionText: gettext('Edit Thumbnail'), icon: '', - text: gettext('Edit Thumbnail') + text: HtmlUtils.interpolateHtml( + // Translators: This is a 2 part text which tells the image requirements. + gettext('{InstructionsSpanStart}{videoImageResoultion}{lineBreak} {videoImageSupportedFileFormats}{spanEnd}'), // eslint-disable-line max-len + { + videoImageResoultion: this.getVideoImageResolution(), + videoImageSupportedFileFormats: this.getVideoImageSupportedFileFormats().humanize, + lineBreak: HtmlUtils.HTML('
'), + InstructionsSpanStart: HtmlUtils.HTML(''), + spanEnd: HtmlUtils.HTML('') + } + ).toString() }, error: { + name: 'error', icon: '', text: gettext('Image upload failed') }, progress: { + name: 'progress-action', icon: '', text: gettext('Uploading') }, requirements: { + name: 'requirements', icon: '', text: HtmlUtils.interpolateHtml( // Translators: This is a 3 part text which tells the image requirements. - gettext('{ReqTextSpanStart}Image requirements{spanEnd}{lineBreak}{InstructionsSpanStart}{videoImageResoultion}{lineBreak} {videoImageSupportedFileFormats}{spanEnd}'), // eslint-disable-line max-len + gettext('{ReqTextSpanStart}Requirements{spanEnd}{lineBreak}{InstructionsSpanStart}{videoImageResoultion}{lineBreak} {videoImageSupportedFileFormats}{spanEnd}'), // eslint-disable-line max-len { videoImageResoultion: this.getVideoImageResolution(), videoImageSupportedFileFormats: this.getVideoImageSupportedFileFormats().humanize, @@ -259,26 +275,40 @@ define( }, setActionInfo: function(action, showText, additionalSRText) { + var hasError = this.$('.thumbnail-wrapper').hasClass('error'); this.$('.thumbnail-action').toggle(showText); - - // In case of error, we don't want to show any icon on the image. - if (action === 'error') { - HtmlUtils.setHtml( - this.$('.thumbnail-action .action-icon'), - HtmlUtils.HTML('') - ); - } else { - HtmlUtils.setHtml( - this.$('.thumbnail-action .action-icon'), - HtmlUtils.HTML(this.actionsInfo[action].icon) - ); - } + HtmlUtils.setHtml( + this.$('.thumbnail-action .action-icon'), + HtmlUtils.HTML(this.actionsInfo[action].icon) + ); HtmlUtils.setHtml( this.$('.thumbnail-action .action-text'), HtmlUtils.HTML(this.actionsInfo[action].text) ); this.$('.thumbnail-action .action-text-sr').text(additionalSRText || ''); this.$('.thumbnail-wrapper').attr('class', 'thumbnail-wrapper {action}'.replace('{action}', action)); + this.$('.thumbnail-action .action-icon') + .attr('class', 'action-icon {action}'.replace('{action}', action)); + + // Add error class if it was already present. + if (hasError) { + this.$('.thumbnail-wrapper').addClass('error'); + } + + // Don't show edit-container layout on progress action. + if (action === 'progress') { + this.$('.thumbnail-action .edit-container').toggle(false); + } else if (action === 'edit') { + this.$('.thumbnail-action .edit-container').toggle(true); + HtmlUtils.setHtml( + this.$('.thumbnail-action .edit-container .action-icon'), + HtmlUtils.HTML(this.actionsInfo[action].icon) + ); + HtmlUtils.setHtml( + this.$('.thumbnail-action .edit-container .edit-action-text'), + HtmlUtils.HTML(this.actionsInfo[action].actionText) + ); + } }, validateImageFile: function(imageFile) { @@ -320,22 +350,29 @@ define( if ($thumbnailWrapperEl.length) { $thumbnailWrapperEl.remove(); } + // Remove error class from thumbnail wrapper as well. + $('.thumbnail-wrapper').removeClass('error'); }, showErrorMessage: function(errorText) { var videoId = this.model.get('edx_video_id'), $parentRowEl = $(this.$el.parent()); - this.action = 'error'; + // If image url is not this.defaultVideoImageURL then it means image is uploaded + // so we should treat it as edit action otherwise default upload action. + this.action = this.$('.thumbnail-wrapper img').attr('src') !== this.defaultVideoImageURL + ? 'edit' : 'upload'; this.setActionInfo(this.action, true); this.readMessages([gettext('Could not upload the video image file'), errorText]); + errorText = gettext('Image upload failed. ') + errorText; // eslint-disable-line no-param-reassign // Add error wrapper html to current video element row. - $parentRowEl.before( // safe-lint: disable=javascript-jquery-insertion + $parentRowEl.before( // xss-lint: disable=javascript-jquery-insertion HtmlUtils.ensureHtml( this.thumbnailErrorTemplate({videoId: videoId, errorText: errorText}) ).toString() ); + this.$el.find('.thumbnail-wrapper').addClass('error'); }, readMessages: function(messages) { diff --git a/cms/static/sass/views/_video-upload.scss b/cms/static/sass/views/_video-upload.scss index 61582063cb..ebc5cc14c1 100644 --- a/cms/static/sass/views/_video-upload.scss +++ b/cms/static/sass/views/_video-upload.scss @@ -179,23 +179,15 @@ display: table-cell; } - .thumbnail-col { - width: 15%; - } - .name-col { width: 25%; } - .date-col { - width: 10%; - } - - .video-id-col { + .thumbnail-col, .video-id-col { width: 15%; } - .status-col { + .date-col, .status-col { width: 10%; } @@ -203,20 +195,18 @@ width: 5%; } - + .video-head-col.thumbnail-col { + width: 17% !important; + } } } .thumbnail-error-wrapper { display: table-row; white-space: nowrap; - - .thumbnail-error-text { - color: $red; - padding: 10px 10px 0; - .action-text { - margin-left: 5px; - } + color: $red; + .icon { + margin: ($baseline*0.75) ($baseline/4) 0 ($baseline/2); } } @@ -250,7 +240,7 @@ font-size: 15px; font-family: "Open Sans"; text-align: left; - color: #4c4c4c; + color: $gray-d2; line-height: 1.5; } .video-duration { @@ -271,7 +261,6 @@ } } - &.error, &.progress { background: white; @@ -285,10 +274,6 @@ } } - &.error .thumbnail-action { - color: $red; - } - &.upload .thumbnail-action { color: $blue; } @@ -299,10 +284,24 @@ } } - &.edit .thumbnail-action { - background-color: white; + &.edit { + background-color: #4e4e4e; + } + + &.edit .thumbnail-action .action-icon.edit { + display: none; + } + + &.edit .thumbnail-action .edit-container { + background-color: $white; padding: ($baseline/4); border-radius: ($baseline/5); + margin-top: ($baseline/2); + display: none; + } + + &.edit .action-text { + color: $white; } .thumbnail-action { @@ -316,6 +315,7 @@ left: 5px;; right: 5px; @include transform(translateY(-50%)); + z-index: 1; } .upload-image-input { @@ -344,5 +344,9 @@ &.focused { box-shadow: 0 0 ($baseline/5) 1px $blue; } + + &.error { + box-shadow: 0 0 ($baseline/5) 1px $red; + } } } diff --git a/cms/templates/js/video-thumbnail-error.underscore b/cms/templates/js/video-thumbnail-error.underscore index 8e0e84a7d6..4f7c7c4d2e 100644 --- a/cms/templates/js/video-thumbnail-error.underscore +++ b/cms/templates/js/video-thumbnail-error.underscore @@ -1,8 +1,4 @@
-
- - <%- errorText %> -
+ + <%- errorText %>
diff --git a/cms/templates/js/video-thumbnail.underscore b/cms/templates/js/video-thumbnail.underscore index 693bc21682..1d65386689 100644 --- a/cms/templates/js/video-thumbnail.underscore +++ b/cms/templates/js/video-thumbnail.underscore @@ -3,9 +3,13 @@
diff --git a/requirements/edx/github.txt b/requirements/edx/github.txt index 5da6cf4748..e4cb8f6202 100644 --- a/requirements/edx/github.txt +++ b/requirements/edx/github.txt @@ -77,7 +77,7 @@ git+https://github.com/edx/lettuce.git@0.2.20.002#egg=lettuce==0.2.20.002 git+https://github.com/edx/edx-ora2.git@1.4.3#egg=ora2==1.4.3 -e git+https://github.com/edx/edx-submissions.git@2.0.0#egg=edx-submissions==2.0.0 git+https://github.com/edx/ease.git@release-2015-07-14#egg=ease==0.1.3 -git+https://github.com/edx/edx-val.git@0.0.15#egg=edxval==0.0.15 +git+https://github.com/edx/edx-val.git@ammar/auto-generated-images-fixes git+https://github.com/pmitros/RecommenderXBlock.git@v1.2#egg=recommender-xblock==1.2 git+https://github.com/solashirai/crowdsourcehinter.git@518605f0a95190949fe77bd39158450639e2e1dc#egg=crowdsourcehinter-xblock==0.1 -e git+https://github.com/pmitros/RateXBlock.git@367e19c0f6eac8a5f002fd0f1559555f8e74bfff#egg=rate-xblock