Merge pull request #11130 from edx/clytwynec/ac-75

AC-75 and AC-73

common/static/common/templates/discussion/customwmd-prompt.underscore

+<h4 id="editor-dialog-title"><%- title %></h4>
+<div role="status" id="wmd-editor-dialog-form-errors" class="sr" tabindex="-1"></div>
+<div>
+    <form class="form">
+        <fieldset class="field-group">
+            <legend class="form-group-hd sr"><%- title %></legend>
+            <div class="field <%- uploadFieldClass %>">
+                <label id="new-url-input-label" for="new-url-input" class="field-label">
+                    <%- urlLabel %></label>
+                <input type="text" id="new-url-input" class="field-input input-text" aria-describedby="new-url-input-help">
+                <% if (imageUploadHandler) { %>
+                    <button id="file-upload-proxy" class="btn btn-primary btn-base form-btn">
+                        <%- chooseFileText %>
+                    </button>
+                    <input type="file" name="file-upload" id="file-upload" style="display:none;"/>
+                <% } %>
+                <div id="new-url-input-field-message" class="field-message has-error" style="display:none">
+                    <span class="field-message-content"><%- urlError %></span>
+                </div>
+                <div id="new-url-input-help" class="field-hint">
+                    <%- urlHelp %>
+                </div>
+            </div>
+            <div class="field">
+                <label for="new-url-desc-input" class="field-label"><%- urlDescLabel %></label>
+                <input type="text" id="new-url-desc-input" class="field-input input-text" required aria-describedby="new-url-desc-input-help">
+                <div id="new-url-desc-input-field-message" class="field-message has-error" style="display:none">
+                    <span class="field-message-content"><%- descError %></span>
+                </div>
+                <div id="new-url-desc-input-help" class="field-hint">
+                    <%- urlDescHelp %>
+                    <% if (urlDescHelpLink) { %>
+                        <a href="<%- urlDescHelpLink['href'] %>">
+                            <%- urlDescHelpLink['text'] %>
+                        </a>
+                    <% } %>
+                </div>
+            </div>
+            <div class="field"><% if (imageUploadHandler) { %>
+                <label for="img-is-decorative" class="field-label label-inline">
+                    <input type="checkbox" id="img-is-decorative" class="field-input input-checkbox">
+                    <span class="field-input-label"><%- imageIsDecorativeLabel %></span>
+                </label> <% }
+            %></div>
+        </fieldset>
+        <div class="form-actions">
+            <input type="button" id="new-link-image-ok" class="btn btn-primary btn-base form-btn" value="<%- okText %>" />
+            <input type="button" id="new-link-image-cancel" class="btn btn-primary btn-base form-btn" value="<%- cancelText %>" >
+        </div>
+    </form>
+</div>

common/test/acceptance/pages/lms/discussion.py

@@ -195,17 +195,58 @@ class DiscussionThreadPage(PageObject, DiscussionPageMixin):
         """Replace the contents of the response editor"""
         self._find_within(".response_{} .discussion-response .wmd-input".format(response_id)).fill(new_body)
 
+    def verify_link_editor_error_messages_shown(self):
+        """
+        Confirm that the error messages are displayed in the editor.
+        """
+        def errors_visible():
+            """
+            Returns True if both errors are visible, False otherwise.
+            """
+            return (
+                self.q(css="#new-url-input-field-message.has-error").visible and
+                self.q(css="#new-url-desc-input-field-message.has-error").visible
+            )
+
+        self.wait_for(errors_visible, "Form errors should be visible.")
+
+    def add_content_via_editor_button(self, content_type, response_id, url, description, is_decorative=False):
+        """Replace the contents of the response editor"""
+        self._find_within(
+            "#wmd-{}-button-edit-post-body-{}".format(
+                content_type,
+                response_id,
+            )
+        ).click()
+        self.q(css='#new-url-input').fill(url)
+        self.q(css='#new-url-desc-input').fill(description)
+
+        if is_decorative:
+            self.q(css='#img-is-decorative').click()
+
+        self.q(css='input[value="OK"]').click()
+
     def submit_response_edit(self, response_id, new_response_body):
         """Click the submit button on the response editor"""
-        self._find_within(".response_{} .discussion-response .post-update".format(response_id)).first.click()
-        EmptyPromise(
-            lambda: (
+
+        def submit_response_check_func():
+            """
+            Tries to click "Update post" and returns True if the post
+            was successfully updated, False otherwise.
+            """
+            self._find_within(
+                ".response_{} .discussion-response .post-update".format(
+                    response_id
+                )
+            ).first.click()
+
+            return (
                 not self.is_response_editor_visible(response_id) and
                 self.is_response_visible(response_id) and
                 self.get_response_body(response_id) == new_response_body
-            ),
-            "Comment edit succeeded"
-        ).fulfill()
+            )
+
+        self.wait_for(submit_response_check_func, "Comment edit succeeded")
 
     def is_show_comments_visible(self, response_id):
         """Returns true if the "show comments" link is visible for a response"""

common/test/acceptance/tests/discussion/test_discussion.py

@@ -449,6 +449,145 @@ class DiscussionResponseEditTest(BaseDiscussionTestCase):
         page.set_response_editor_value(response_id, new_response)
         page.submit_response_edit(response_id, new_response)
 
+    def test_edit_response_add_link(self):
+        """
+        Scenario: User submits valid input to the 'add link' form
+            Given I am editing a response on a discussion page
+            When I click the 'add link' icon in the editor toolbar
+            And enter a valid url to the URL input field
+            And enter a valid string in the Description input field
+            And click the 'OK' button
+            Then the edited response should contain the new link
+        """
+        self.setup_user()
+        self.setup_view()
+        page = self.create_single_thread_page("response_edit_test_thread")
+        page.visit()
+
+        response_id = "response_self_author"
+        url = "http://example.com"
+        description = "example"
+
+        page.start_response_edit(response_id)
+        page.set_response_editor_value(response_id, "")
+        page.add_content_via_editor_button(
+            "link", response_id, url, description)
+        page.submit_response_edit(response_id, description)
+
+        expected_response_html = (
+            '<p><a href="{}">{}</a></p>'.format(url, description)
+        )
+        actual_response_html = page.q(
+            css=".response_{} .response-body".format(response_id)
+        ).html[0]
+        self.assertEqual(expected_response_html, actual_response_html)
+
+    def test_edit_response_add_image(self):
+        """
+        Scenario: User submits valid input to the 'add image' form
+            Given I am editing a response on a discussion page
+            When I click the 'add image' icon in the editor toolbar
+            And enter a valid url to the URL input field
+            And enter a valid string in the Description input field
+            And click the 'OK' button
+            Then the edited response should contain the new image
+        """
+        self.setup_user()
+        self.setup_view()
+        page = self.create_single_thread_page("response_edit_test_thread")
+        page.visit()
+
+        response_id = "response_self_author"
+        url = "http://www.example.com/something.png"
+        description = "image from example.com"
+
+        page.start_response_edit(response_id)
+        page.set_response_editor_value(response_id, "")
+        page.add_content_via_editor_button(
+            "image", response_id, url, description)
+        page.submit_response_edit(response_id, '')
+
+        expected_response_html = (
+            '<p><img src="{}" alt="{}" title=""></p>'.format(url, description)
+        )
+        actual_response_html = page.q(
+            css=".response_{} .response-body".format(response_id)
+        ).html[0]
+        self.assertEqual(expected_response_html, actual_response_html)
+
+    def test_edit_response_add_image_error_msg(self):
+        """
+        Scenario: User submits invalid input to the 'add image' form
+            Given I am editing a response on a discussion page
+            When I click the 'add image' icon in the editor toolbar
+            And enter an invalid url to the URL input field
+            And enter an empty string in the Description input field
+            And click the 'OK' button
+            Then I should be shown 2 error messages
+        """
+        self.setup_user()
+        self.setup_view()
+        page = self.create_single_thread_page("response_edit_test_thread")
+        page.visit()
+        page.start_response_edit("response_self_author")
+        page.add_content_via_editor_button(
+            "image", "response_self_author", '', '')
+        page.verify_link_editor_error_messages_shown()
+
+    def test_edit_response_add_decorative_image(self):
+        """
+        Scenario: User submits invalid input to the 'add image' form
+            Given I am editing a response on a discussion page
+            When I click the 'add image' icon in the editor toolbar
+            And enter a valid url to the URL input field
+            And enter an empty string in the Description input field
+            And I check the 'image is decorative' checkbox
+            And click the 'OK' button
+            Then the edited response should contain the new image
+        """
+        self.setup_user()
+        self.setup_view()
+        page = self.create_single_thread_page("response_edit_test_thread")
+        page.visit()
+
+        response_id = "response_self_author"
+        url = "http://www.example.com/something.png"
+        description = ""
+
+        page.start_response_edit(response_id)
+        page.set_response_editor_value(response_id, "Some content")
+        page.add_content_via_editor_button(
+            "image", response_id, url, description, is_decorative=True)
+        page.submit_response_edit(response_id, "Some content")
+
+        expected_response_html = (
+            '<p>Some content<img src="{}" alt="{}" title=""></p>'.format(
+                url, description)
+        )
+        actual_response_html = page.q(
+            css=".response_{} .response-body".format(response_id)
+        ).html[0]
+        self.assertEqual(expected_response_html, actual_response_html)
+
+    def test_edit_response_add_link_error_msg(self):
+        """
+        Scenario: User submits invalid input to the 'add link' form
+            Given I am editing a response on a discussion page
+            When I click the 'add link' icon in the editor toolbar
+            And enter an invalid url to the URL input field
+            And enter an empty string in the Description input field
+            And click the 'OK' button
+            Then I should be shown 2 error messages
+        """
+        self.setup_user()
+        self.setup_view()
+        page = self.create_single_thread_page("response_edit_test_thread")
+        page.visit()
+        page.start_response_edit("response_self_author")
+        page.add_content_via_editor_button(
+            "link", "response_self_author", '', '')
+        page.verify_link_editor_error_messages_shown()
+
     def test_edit_response_as_student(self):
         """
         Scenario: Students should be able to edit the response they created not responses of other users

lms/static/js/spec/main.js

@@ -221,7 +221,7 @@
                 exports: 'Markdown.Converter'
             },
             'Markdown.Editor': {
-                deps: ['Markdown.Converter'],
+                deps: ['Markdown.Converter', 'gettext', 'underscore'],
                 exports: 'Markdown.Editor'
             },
             'Markdown.Sanitizer': {
@@ -744,7 +744,8 @@
         'lms/include/js/spec/financial-assistance/financial_assistance_form_view_spec.js',
         'lms/include/js/spec/bookmarks/bookmarks_list_view_spec.js',
         'lms/include/js/spec/bookmarks/bookmark_button_view_spec.js',
-        'lms/include/js/spec/views/message_banner_spec.js'
+        'lms/include/js/spec/views/message_banner_spec.js',
+        'lms/include/js/spec/markdown_editor_spec.js'
     ]);
 
 }).call(this, requirejs, define);

lms/static/js/spec/markdown_editor_spec.js

+define(['Markdown.Editor'], function(MarkdownEditor) {
+    'use strict';
+    describe('Markdown.Editor', function() {
+        var editor = new MarkdownEditor();
+
+        describe('util.isValidUrl', function () {
+            it('should return true for http://example.com', function () {
+                expect(
+                    editor.util.isValidUrl('http://example.com')
+                ).toBeTruthy();
+            });
+            it('should return true for https://example.com', function () {
+                expect(
+                    editor.util.isValidUrl('https://example.com')
+                ).toBeTruthy();
+            });
+            it('should return true for ftp://example.com', function () {
+                expect(
+                    editor.util.isValidUrl('ftp://example.com')
+                ).toBeTruthy();
+            });
+            it('should return false for http://', function () {
+                expect(editor.util.isValidUrl('http://')).toBeFalsy();
+            });
+            it('should return false for https://', function () {
+                expect(editor.util.isValidUrl('https://')).toBeFalsy();
+            });
+            it('should return false for ftp://', function () {
+                expect(editor.util.isValidUrl('ftp://')).toBeFalsy();
+            });
+            it('should return false for fake://example.com', function () {
+                expect(
+                    editor.util.isValidUrl('fakeprotocol://example.com')
+                ).toBeFalsy();
+            });
+            it('should return false for fake://', function () {
+                expect(
+                    editor.util.isValidUrl('fakeprotocol://')
+                ).toBeFalsy();
+            });
+            it('should return false for www.noprotocol.com', function () {
+                expect(
+                    editor.util.isValidUrl('www.noprotocol.com')
+                ).toBeFalsy();
+            });
+            it('should return false for an empty string', function () {
+                expect(
+                    editor.util.isValidUrl('')
+                ).toBeFalsy();
+            });
+        });
+    });
+});

lms/static/sass/_build-course.scss

@@ -59,6 +59,7 @@
 
 // discussion
 @import "course/discussion/form-wmd-toolbar";
+@import "course/discussion/form";
 
 // search
 @import 'search/_search';

lms/static/sass/course/discussion/_form-wmd-toolbar.scss

@@ -105,16 +105,38 @@
   font-family: arial, helvetica, sans-serif;
 }
 
-
-.wmd-prompt-dialog > form > input[type="text"] {
-  border: 1px solid #999999;
-  color: black;
-}
-
-.wmd-prompt-dialog > form > input[type="button"]{
-  border: 1px solid #888888;
-  font-family: trebuchet MS, helvetica, sans-serif;
-  font-size: 1em;
-  font-weight: bold;
+.wmd-prompt-dialog {
+  .form {
+    width: 400px;
+    margin: 0 auto;
+  }
+  .form-actions {
+    text-align: center;
+  }
+
+  .input-text {
+    width: 400px;
+    padding: 0px 12px;
+  }
+
+  .field-hint,
+  .field-message.has-error {
+    width: 380px;
+  }
+
+  .file-upload {
+    .input-text {
+      width: 230px;
+    }
+    .field-message.has-error {
+      margin-top: -1px;
+      width: 210px;
+    }
+    .form-btn {
+      margin: 0 0 0 10px;
+      height: 35px;
+      width: 130px;
+      padding: 6px;
+    }
+  }
 }
-

lms/static/sass/course/discussion/_form.scss

+// ------------------------------
+// edX Pattern Library: Components - Forms
+
+// About: Contains base styling for forms
+
+// #SETTINGS
+// #GLOBAL
+// #INPUT TEXT
+// #INPUT RADIO/CHECKBOX
+// ------------------------------
+
+
+// ------------------------------
+// IMPORTANT: This is meant for the modals in the discussion forum
+// when doing updates on them we added the classes to make it compatible
+// with the pattern library. So, it is modified and scoped to that modal.
+
+// TODO: Remove this file once the pattern library it implemented.
+// ------------------------------
+.wmd-prompt-dialog {
+    // ----------------------------
+    // #SETTINGS
+    // ----------------------------
+    $spacing-vertical-x-small:                  ($baseline/2);
+    $spacing-vertical-base:                     ($baseline*2);
+    $spacing-vertical-mid-small:                ($baseline*1.5);
+    $spacing-vertical-small:                    $baseline;
+
+    $font-size-large:                           18px;
+    $font-size-base:                            16px;
+    $font-size-small:                           14px;
+
+    $component-border-radius:                   3px !default;
+
+    $error-base:                                rgb(178, 6, 16);
+    $error-dark:                                rgb(125, 9, 16);
+    $grayscale-x-dark:                          rgb(77, 75, 75);
+    $grayscale-x-light:                         rgb(231, 230, 230);
+    $grayscale-white:                           rgb(252, 252, 252);
+    $grayscale-cool-x-dark:                     rgb(52, 56, 58);
+    $grayscale-cool-x-light:                    rgb(229, 233, 235);
+    $primary-accent:                            rgb(14, 166, 236);
+    $transparent:                               rgba(167, 164, 164, 0.498039);
+
+    $text-base-color:                           $grayscale-x-dark !default;
+    $label-color:                               $text-base-color !default;
+    $label-color-active:                        $grayscale-x-dark !default;
+    $input-placeholder-text:                    $grayscale-cool-x-light !default;
+    $input-default-background:                  $grayscale-white !default;
+    $input-default-border-color:                $grayscale-x-light !default;
+    $input-default-focus-border-color:          $primary-accent !default;
+    $input-default-color:                       $grayscale-cool-x-dark !default;
+    $input-default-focus-color:                 $grayscale-cool-x-dark !default;
+    $input-alt-background:                      $transparent !default;
+    $input-alt-focus-border-color:              $grayscale-x-dark !default;
+
+
+    // ----------------------------
+    // #GLOBAL
+    // ----------------------------
+    // sections of a form
+    .form-group {
+        margin-bottom: $spacing-vertical-mid-small;
+
+        // section title or legend
+        .form-group-hd {
+            margin-bottom: $spacing-vertical-small;
+            font-size: $font-size-large;
+        }
+
+        .field {
+            margin-bottom: $spacing-vertical-base;
+
+            &:last-child {
+                margin-bottom: 0;
+            }
+        }
+    }
+
+    // radio button and checkbox fieldsets
+    .field-group {
+        margin-bottom: $spacing-vertical-small;
+
+        // group title or legend
+        .field-group-hd {
+            margin-bottom: $spacing-vertical-small;
+            font-size: $font-size-large;
+        }
+
+        .field {
+            margin-bottom: $spacing-vertical-x-small;
+
+            &:last-child {
+                margin-bottom: 0;
+            }
+        }
+    }
+
+    .field-label {
+        display: block;
+        width: auto;
+        margin-bottom: $spacing-vertical-x-small;
+        font-size: $font-size-base;
+        line-height: 100%;
+        color: $label-color;
+
+        // presents the label inline with the form control
+        &.label-inline {
+            display: inline-block;
+            margin-bottom: 0;
+        }
+
+        // STATE: is selected
+        .field-input:checked + .field-input-label,
+        .field-radio:checked + .field-input-label,
+        &.is-active,
+        &.is-selected {
+            color: $label-color-active;
+        }
+    }
+
+    .field-message {
+        font-size: $font-size-small;
+        border-bottom-left-radius: $component-border-radius;
+        border-bottom-right-radius: $component-border-radius;
+
+        &.has-error {
+            padding: $spacing-vertical-x-small;
+            background: $error-base;
+            color: $grayscale-white;
+        }
+    }
+
+    .field-input,
+    .field-select,
+    .field-textarea {
+        display: inline-block;
+        padding: rem($baseline/2);
+        border: 1px solid $input-default-border-color;
+        background: $input-default-background;
+        font-size: $font-size-base;
+        color: $input-default-color;
+
+        // STATE: is active or has focus
+        &:focus,
+        &.is-active {
+            border-color: $input-default-focus-border-color;
+            color: $input-default-focus-color;
+        }
+
+        // STATE: has an error
+        &.has-error {
+            border-color: $error-base;
+
+            & + .field-hint {
+                color: $error-dark;
+            }
+
+            .icon {
+                fill: $error-base;
+            }
+        }
+    }
+
+
+    // ----------------------------
+    // #INPUT TEXT
+    // ----------------------------
+    .input-text {
+
+        &.input-alt {
+            padding: $spacing-vertical-small 0;
+            border-width: 0 0 2px 0;
+            background: $input-alt-background;
+
+            // STATE: is active or has focus
+            &:focus,
+            &.is-active {
+                border-color: $input-alt-focus-border-color;
+                background: $input-alt-background;
+            }
+
+            &.has-error {
+                border-width: 1px 1px 2px 1px;
+                border-color: $error-base;
+            }
+        }
+    }
+
+    // ----------------------------
+    // Buttons
+    // ----------------------------
+    .form-btn {
+        display: inline-block;
+        margin: 10px;
+        border-style: solid;
+        border-radius: $component-border-radius;
+        border-width: 1px;
+        padding: $spacing-vertical-x-small $baseline;
+        font-size: $font-size-base;
+        font-weight: 600;
+    }
+}

lms/templates/discussion/_underscore_templates.html

@@ -11,7 +11,7 @@ template_names = [
     'thread', 'thread-show', 'thread-edit', 'thread-response', 'thread-response-show', 'thread-response-edit',
     'response-comment-show', 'response-comment-edit', 'thread-list-item', 'discussion-home', 'search-alert',
     'new-post', 'thread-type', 'new-post-menu-entry', 'new-post-menu-category', 'topic', 'post-user-display',
-    'inline-discussion', 'pagination', 'user-profile', 'profile-thread'
+    'inline-discussion', 'pagination', 'user-profile', 'profile-thread', 'customwmd-prompt'
 ]
 %>