Use static onload handlers in the image chooser modal

Instead of passing an 'onload' JS function as part of the AJAX response for each step of the workflow,
we specify all onload handlers up-front when initialising ModalWorkflow, and return a 'step' field
in the response to indicate which one to trigger.

.eslintignore

@@ -17,10 +17,7 @@ wagtail/admin/templates/wagtailadmin/edit_handlers/inline_panel.js
 wagtail/contrib/search_promotions/templates/wagtailsearchpromotions/includes/searchpromotions_formset.js
 wagtail/users/templates/wagtailusers/groups/includes/page_permissions_formset.js
 wagtail/snippets/templates/wagtailsnippets/chooser/chosen.js
-wagtail/images/templates/wagtailimages/chooser/image_chosen.js
-wagtail/images/templates/wagtailimages/chooser/chooser.js
 wagtail/search/templates/wagtailsearch/queries/chooser/chooser.js
-wagtail/images/templates/wagtailimages/chooser/select_format.js
 wagtail/embeds/templates/wagtailembeds/chooser/embed_chosen.js
 wagtail/embeds/templates/wagtailembeds/chooser/chooser.js
 wagtail/documents/templates/wagtaildocs/chooser/chooser.js

client/src/components/Draftail/sources/ModalWorkflowSource.js

@@ -26,12 +26,14 @@ export const getChooserConfig = (entityType, entity, selectedText) => {
     return {
       url: `${global.chooserUrls.imageChooser}?select_format=true`,
       urlParams: {},
+      onload: global.IMAGE_CHOOSER_MODAL_ONLOAD_HANDLERS,
     };
 
   case EMBED:
     return {
       url: global.chooserUrls.embedsChooser,
       urlParams: {},
+      onload: {},
     };
 
   case ENTITY_TYPE.LINK:
@@ -61,18 +63,21 @@ export const getChooserConfig = (entityType, entity, selectedText) => {
     return {
       url,
       urlParams,
+      onload: {},
     };
 
   case DOCUMENT:
     return {
       url: global.chooserUrls.documentChooser,
       urlParams: {},
+      onload: {},
     };
 
   default:
     return {
       url: null,
       urlParams: {},
+      onload: {},
     };
   }
 };
@@ -133,7 +138,7 @@ class ModalWorkflowSource extends Component {
   componentDidMount() {
     const { onClose, entityType, entity, editorState } = this.props;
     const selectedText = getSelectionText(editorState);
-    const { url, urlParams } = getChooserConfig(entityType, entity, selectedText);
+    const { url, urlParams, onload } = getChooserConfig(entityType, entity, selectedText);
 
     $(document.body).on('hidden.bs.modal', this.onClose);
 
@@ -141,6 +146,7 @@ class ModalWorkflowSource extends Component {
     this.workflow = global.ModalWorkflow({
       url,
       urlParams,
+      onload,
       responses: {
         imageChosen: this.onChosen,
         // Discard the first parameter (HTML) to only transmit the data.

client/src/components/Draftail/sources/ModalWorkflowSource.test.js

@@ -34,6 +34,7 @@ describe('ModalWorkflowSource', () => {
       expect(getChooserConfig({ type: 'IMAGE' }, null, '')).toEqual({
         url: '/admin/images/chooser/?select_format=true',
         urlParams: {},
+        onload: global.IMAGE_CHOOSER_MODAL_ONLOAD_HANDLERS,
       });
     });
 
@@ -41,6 +42,7 @@ describe('ModalWorkflowSource', () => {
       expect(getChooserConfig({ type: 'EMBED' }, null, '')).toEqual({
         url: '/admin/embeds/chooser/',
         urlParams: {},
+        onload: {},
       });
     });
 
@@ -48,6 +50,7 @@ describe('ModalWorkflowSource', () => {
       expect(getChooserConfig({ type: 'DOCUMENT' }, null, '')).toEqual({
         url: '/admin/documents/chooser/',
         urlParams: {},
+        onload: {},
       });
     });
 

client/src/components/Draftail/sources/__snapshots__/ModalWorkflowSource.test.js.snap

@@ -50,6 +50,7 @@ Object {
 
 exports[`ModalWorkflowSource #getChooserConfig LINK external 1`] = `
 Object {
+  "onload": Object {},
   "url": "/admin/choose-external-link/",
   "urlParams": Object {
     "allow_email_link": true,
@@ -64,6 +65,7 @@ Object {
 
 exports[`ModalWorkflowSource #getChooserConfig LINK mail 1`] = `
 Object {
+  "onload": Object {},
   "url": "/admin/choose-email-link/",
   "urlParams": Object {
     "allow_email_link": true,
@@ -78,6 +80,7 @@ Object {
 
 exports[`ModalWorkflowSource #getChooserConfig LINK no entity 1`] = `
 Object {
+  "onload": Object {},
   "url": "/admin/choose-page/",
   "urlParams": Object {
     "allow_email_link": true,
@@ -91,6 +94,7 @@ Object {
 
 exports[`ModalWorkflowSource #getChooserConfig LINK page 1`] = `
 Object {
+  "onload": Object {},
   "url": "/admin/choose-page/0/",
   "urlParams": Object {
     "allow_email_link": true,

client/tests/stubs.js

@@ -58,6 +58,8 @@ global.chooserUrls = {
   snippetChooser: '/admin/snippets/choose/',
 };
 
+global.IMAGE_CHOOSER_MODAL_ONLOAD_HANDLERS = {};
+
 const jQueryObj = {
   on: jest.fn(),
   off: jest.fn(),

wagtail/admin/static_src/wagtailadmin/js/modal-workflow.js

@@ -8,6 +8,9 @@ function ModalWorkflow(opts) {
         'url' (required): initial
         'responses' (optional): dict of callbacks to be called when the modal content
             calls modal.respond(callbackName, params)
+        'onload' (optional): dict of callbacks to be called when loading a step of the workflow.
+            The 'step' field in the response identifies the callback to call, passing it the
+            modal object and response data as arguments
     */
 
     var self = {};
@@ -65,10 +68,16 @@ function ModalWorkflow(opts) {
         }
 
         if (response.onload) {
-            // if response contains an 'onload' funtion, call it
+            // if response contains an 'onload' function, call it
             // (passing this modal object and the full response data)
             response.onload(self, response);
         }
+
+        /* If response contains a 'step' identifier, and that identifier is found in
+        the onload dict, call that onload handler */
+        if (opts.onload && response.step && (response.step in opts.onload)) {
+            opts.onload[response.step](self, response);
+        }
     };
 
     self.respond = function(responseType) {

wagtail/images/static_src/wagtailimages/js/hallo-plugins/hallo-wagtailimage.js

@@ -26,6 +26,7 @@
                     insertionPoint = $(lastSelection.endContainer).parentsUntil('[data-hallo-editor]').last();
                     return ModalWorkflow({
                         url: window.chooserUrls.imageChooser + '?select_format=true',
+                        onload: IMAGE_CHOOSER_MODAL_ONLOAD_HANDLERS,
                         responses: {
                             imageChosen: function(imageData) {
                                 var elem;

wagtail/images/static_src/wagtailimages/js/image-chooser-modal.js

@@ -0,0 +1,148 @@
+IMAGE_CHOOSER_MODAL_ONLOAD_HANDLERS = {
+    'chooser': function(modal, jsonData) {
+        var searchUrl = $('form.image-search', modal.body).attr('action');
+
+        /* currentTag stores the tag currently being filtered on, so that we can
+        preserve this when paginating */
+        var currentTag;
+
+        function ajaxifyLinks (context) {
+            $('.listing a', context).on('click', function() {
+                modal.loadUrl(this.href);
+                return false;
+            });
+
+            $('.pagination a', context).on('click', function() {
+                var page = this.getAttribute("data-page");
+                setPage(page);
+                return false;
+            });
+        }
+
+        function fetchResults(requestData) {
+            $.ajax({
+                url: searchUrl,
+                data: requestData,
+                success: function(data, status) {
+                    $('#image-results').html(data);
+                    ajaxifyLinks($('#image-results'));
+                }
+            });
+        }
+
+        function search() {
+            /* Searching causes currentTag to be cleared - otherwise there's
+            no way to de-select a tag */
+            currentTag = null;
+            fetchResults({
+                q: $('#id_q').val(),
+                collection_id: $('#collection_chooser_collection_id').val()
+            });
+            return false;
+        }
+
+        function setPage(page) {
+            params = {p: page};
+            if ($('#id_q').val().length){
+                params['q'] = $('#id_q').val();
+            }
+            if (currentTag) {
+                params['tag'] = currentTag;
+            }
+            params['collection_id'] = $('#collection_chooser_collection_id').val();
+            fetchResults(params);
+            return false;
+        }
+
+        ajaxifyLinks(modal.body);
+
+        $('form.image-upload', modal.body).on('submit', function() {
+            var formdata = new FormData(this);
+
+            if ($('#id_title', modal.body).val() == '') {
+                var li = $('#id_title', modal.body).closest('li');
+                if (!li.hasClass('error')) {
+                    li.addClass('error');
+                    $('#id_title', modal.body).closest('.field-content').append('<p class="error-message"><span>This field is required.</span></p>')
+                }
+                setTimeout(cancelSpinner, 500);
+            } else {
+                $.ajax({
+                    url: this.action,
+                    data: formdata,
+                    processData: false,
+                    contentType: false,
+                    type: 'POST',
+                    dataType: 'text',
+                    success: function(response){
+                        modal.loadResponseText(response);
+                    },
+                    error: function(response, textStatus, errorThrown) {
+                        message = jsonData['error_message'] + '<br />' + errorThrown + ' - ' + response.status;
+                        $('#upload').append(
+                            '<div class="help-block help-critical">' +
+                            '<strong>' + jsonData['error_label'] + ': </strong>' + message + '</div>');
+                    }
+                });
+            }
+
+            return false;
+        });
+
+        $('form.image-search', modal.body).on('submit', search);
+
+        $('#id_q').on('input', function() {
+            clearTimeout($.data(this, 'timer'));
+            var wait = setTimeout(search, 200);
+            $(this).data('timer', wait);
+        });
+        $('#collection_chooser_collection_id').on('change', search);
+        $('a.suggested-tag').on('click', function() {
+            currentTag = $(this).text();
+            $('#id_q').val('');
+            fetchResults({
+                'tag': currentTag,
+                collection_id: $('#collection_chooser_collection_id').val()
+            });
+            return false;
+        });
+
+        function populateTitle(context) {
+            // Note: There are two inputs with `#id_title` on the page.
+            // The page title and image title. Select the input inside the modal body.
+            var fileWidget = $('#id_file', context);
+            fileWidget.on('change', function () {
+                var titleWidget = $('#id_title', context);
+                var title = titleWidget.val();
+                if (title === '') {
+                    // The file widget value example: `C:\fakepath\image.jpg`
+                    var parts = fileWidget.val().split('\\');
+                    var fileName = parts[parts.length - 1];
+                    titleWidget.val(fileName);
+                }
+            });
+        }
+
+        populateTitle(modal.body);
+
+        /* Add tag entry interface (with autocompletion) to the tag field of the image upload form */
+        $('#id_tags', modal.body).tagit({
+            autocomplete: {source: jsonData['tag_autocomplete_url']}
+        });
+    },
+    'image_chosen': function(modal, jsonData) {
+        modal.respond('imageChosen', jsonData['result']);
+        modal.close();
+    },
+    'select_format': function(modal) {
+        $('form', modal.body).on('submit', function() {
+            var formdata = new FormData(this);
+
+            $.post(this.action, $(this).serialize(), function(response){
+                modal.loadResponseText(response);
+            }, 'text');
+
+            return false;
+        });
+    }
+};

wagtail/images/static_src/wagtailimages/js/image-chooser.js

@@ -7,6 +7,7 @@ function createImageChooser(id) {
     $('.action-choose', chooserElement).on('click', function() {
         ModalWorkflow({
             url: window.chooserUrls.imageChooser,
+            onload: IMAGE_CHOOSER_MODAL_ONLOAD_HANDLERS,
             responses: {
                 imageChosen: function(imageData) {
                     input.val(imageData.id);

wagtail/images/templates/wagtailimages/chooser/chooser.js

@@ -1,131 +0,0 @@
-function(modal, jsonData) {
-    var searchUrl = $('form.image-search', modal.body).attr('action');
-
-    /* currentTag stores the tag currently being filtered on, so that we can
-    preserve this when paginating */
-    var currentTag;
-
-    function ajaxifyLinks (context) {
-        $('.listing a', context).on('click', function() {
-            modal.loadUrl(this.href);
-            return false;
-        });
-
-        $('.pagination a', context).on('click', function() {
-            var page = this.getAttribute("data-page");
-            setPage(page);
-            return false;
-        });
-    }
-
-    function fetchResults(requestData) {
-        $.ajax({
-            url: searchUrl,
-            data: requestData,
-            success: function(data, status) {
-                $('#image-results').html(data);
-                ajaxifyLinks($('#image-results'));
-            }
-        });
-    }
-
-    function search() {
-        /* Searching causes currentTag to be cleared - otherwise there's
-        no way to de-select a tag */
-        currentTag = null;
-        fetchResults({
-            q: $('#id_q').val(),
-            collection_id: $('#collection_chooser_collection_id').val()
-        });
-        return false;
-    }
-
-    function setPage(page) {
-        params = {p: page};
-        if ($('#id_q').val().length){
-            params['q'] = $('#id_q').val();
-        }
-        if (currentTag) {
-            params['tag'] = currentTag;
-        }
-        params['collection_id'] = $('#collection_chooser_collection_id').val();
-        fetchResults(params);
-        return false;
-    }
-
-    ajaxifyLinks(modal.body);
-
-    $('form.image-upload', modal.body).on('submit', function() {
-        var formdata = new FormData(this);
-
-        if ($('#id_title', modal.body).val() == '') {
-            var li = $('#id_title', modal.body).closest('li');
-            if (!li.hasClass('error')) {
-                li.addClass('error');
-                $('#id_title', modal.body).closest('.field-content').append('<p class="error-message"><span>This field is required.</span></p>')
-            }
-            setTimeout(cancelSpinner, 500);
-        } else {
-            $.ajax({
-                url: this.action,
-                data: formdata,
-                processData: false,
-                contentType: false,
-                type: 'POST',
-                dataType: 'text',
-                success: function(response){
-                    modal.loadResponseText(response);
-                },
-                error: function(response, textStatus, errorThrown) {
-                    message = jsonData['error_message'] + '<br />' + errorThrown + ' - ' + response.status;
-                    $('#upload').append(
-                        '<div class="help-block help-critical">' +
-                        '<strong>' + jsonData['error_label'] + ': </strong>' + message + '</div>');
-                }
-            });
-        }
-
-        return false;
-    });
-
-    $('form.image-search', modal.body).on('submit', search);
-
-    $('#id_q').on('input', function() {
-        clearTimeout($.data(this, 'timer'));
-        var wait = setTimeout(search, 200);
-        $(this).data('timer', wait);
-    });
-    $('#collection_chooser_collection_id').on('change', search);
-    $('a.suggested-tag').on('click', function() {
-        currentTag = $(this).text();
-        $('#id_q').val('');
-        fetchResults({
-            'tag': currentTag,
-            collection_id: $('#collection_chooser_collection_id').val()
-        });
-        return false;
-    });
-
-    function populateTitle(context) {
-        // Note: There are two inputs with `#id_title` on the page.
-        // The page title and image title. Select the input inside the modal body.
-        var fileWidget = $('#id_file', context);
-        fileWidget.on('change', function () {
-            var titleWidget = $('#id_title', context);
-            var title = titleWidget.val();
-            if (title === '') {
-                // The file widget value example: `C:\fakepath\image.jpg`
-                var parts = fileWidget.val().split('\\');
-                var fileName = parts[parts.length - 1];
-                titleWidget.val(fileName);
-            }
-        });
-    }
-
-    populateTitle(modal.body);
-
-    /* Add tag entry interface (with autocompletion) to the tag field of the image upload form */
-    $('#id_tags', modal.body).tagit({
-        autocomplete: {source: jsonData['tag_autocomplete_url']}
-    });
-}

wagtail/images/templates/wagtailimages/chooser/image_chosen.js

@@ -1,4 +0,0 @@
-function(modal, jsonData) {
-    modal.respond('imageChosen', jsonData['result']);
-    modal.close();
-}

wagtail/images/templates/wagtailimages/chooser/select_format.js

@@ -1,11 +0,0 @@
-function(modal) {
-    $('form', modal.body).on('submit', function() {
-        var formdata = new FormData(this);
-
-        $.post(this.action, $(this).serialize(), function(response){
-            modal.loadResponseText(response);
-        }, 'text');
-
-        return false;
-    });
-}

wagtail/images/tests/test_admin_views.py

@@ -1,5 +1,4 @@
 import json
-import re
 
 from django.contrib.auth import get_user_model
 from django.contrib.auth.models import Group, Permission
@@ -539,8 +538,9 @@ class TestImageChooserView(TestCase, WagtailTestUtils):
     def test_simple(self):
         response = self.get()
         self.assertEqual(response.status_code, 200)
+        response_json = json.loads(response.content.decode())
+        self.assertEqual(response_json['step'], 'chooser')
         self.assertTemplateUsed(response, 'wagtailimages/chooser/chooser.html')
-        self.assertTemplateUsed(response, 'wagtailimages/chooser/chooser.js')
 
     def test_search(self):
         response = self.get({'q': "Hello"})
@@ -630,7 +630,9 @@ class TestImageChooserChosenView(TestCase, WagtailTestUtils):
     def test_simple(self):
         response = self.get()
         self.assertEqual(response.status_code, 200)
-        self.assertTemplateUsed(response, 'wagtailimages/chooser/image_chosen.js')
+
+        response_json = json.loads(response.content.decode())
+        self.assertEqual(response_json['step'], 'image_chosen')
 
 
 class TestImageChooserSelectFormatView(TestCase, WagtailTestUtils):
@@ -652,8 +654,9 @@ class TestImageChooserSelectFormatView(TestCase, WagtailTestUtils):
     def test_simple(self):
         response = self.get()
         self.assertEqual(response.status_code, 200)
+        response_json = json.loads(response.content.decode())
+        self.assertEqual(response_json['step'], 'select_format')
         self.assertTemplateUsed(response, 'wagtailimages/chooser/select_format.html')
-        self.assertTemplateUsed(response, 'wagtailimages/chooser/select_format.js')
 
     def test_with_edit_params(self):
         response = self.get(params={'alt_text': "some previous alt text"})
@@ -666,16 +669,15 @@ class TestImageChooserSelectFormatView(TestCase, WagtailTestUtils):
         self.assertEqual(response.status_code, 200)
         self.assertEqual(response['Content-Type'], 'text/javascript')
 
-        # extract data as json from the 'result' field
-        match = re.search(r'"result":\s*(.*)}$', response.content.decode())
-        self.assertTrue(match)
-        response_json = json.loads(match.group(1))
+        response_json = json.loads(response.content.decode())
+        self.assertEqual(response_json['step'], 'image_chosen')
+        result = response_json['result']
 
-        self.assertEqual(response_json['id'], self.image.id)
-        self.assertEqual(response_json['title'], "Test image")
-        self.assertEqual(response_json['format'], 'left')
-        self.assertEqual(response_json['alt'], 'Arthur "two sheds" Jackson')
-        self.assertIn('alt="Arthur "two sheds" Jackson"', response_json['html'])
+        self.assertEqual(result['id'], self.image.id)
+        self.assertEqual(result['title'], "Test image")
+        self.assertEqual(result['format'], 'left')
+        self.assertEqual(result['alt'], 'Arthur "two sheds" Jackson')
+        self.assertIn('alt="Arthur "two sheds" Jackson"', result['html'])
 
 
 class TestImageChooserUploadView(TestCase, WagtailTestUtils):
@@ -689,7 +691,8 @@ class TestImageChooserUploadView(TestCase, WagtailTestUtils):
         response = self.get()
         self.assertEqual(response.status_code, 200)
         self.assertTemplateUsed(response, 'wagtailimages/chooser/chooser.html')
-        self.assertTemplateUsed(response, 'wagtailimages/chooser/chooser.js')
+        response_json = json.loads(response.content.decode())
+        self.assertEqual(response_json['step'], 'chooser')
 
     def test_upload(self):
         response = self.client.post(reverse('wagtailimages:chooser_upload'), {

wagtail/images/views/chooser.py

@@ -20,6 +20,7 @@ permission_checker = PermissionPolicyChecker(permission_policy)
 def get_chooser_context():
     """construct context variables needed by the chooser JS"""
     return {
+        'step': 'chooser',
         'error_label': _("Server Error"),
         'error_message': _("Report this error to your webmaster with the following information:"),
         'tag_autocomplete_url': reverse('wagtailadmin_tag_autocomplete'),
@@ -102,7 +103,7 @@ def chooser(request):
 
         paginator, images = paginate(request, images, per_page=12)
 
-        return render_modal_workflow(request, 'wagtailimages/chooser/chooser.html', 'wagtailimages/chooser/chooser.js', {
+        return render_modal_workflow(request, 'wagtailimages/chooser/chooser.html', None, {
             'images': images,
             'uploadform': uploadform,
             'searchform': searchform,
@@ -118,8 +119,8 @@ def image_chosen(request, image_id):
     image = get_object_or_404(get_image_model(), id=image_id)
 
     return render_modal_workflow(
-        request, None, 'wagtailimages/chooser/image_chosen.js',
-        None, json_data={'result': get_image_result_data(image)}
+        request, None, None,
+        None, json_data={'step': 'image_chosen', 'result': get_image_result_data(image)}
     )
 
 
@@ -150,14 +151,14 @@ def chooser_upload(request):
             if request.GET.get('select_format'):
                 form = ImageInsertionForm(initial={'alt_text': image.default_alt_text})
                 return render_modal_workflow(
-                    request, 'wagtailimages/chooser/select_format.html', 'wagtailimages/chooser/select_format.js',
-                    {'image': image, 'form': form}
+                    request, 'wagtailimages/chooser/select_format.html', None,
+                    {'image': image, 'form': form}, json_data={'step': 'select_format'}
                 )
             else:
                 # not specifying a format; return the image details now
                 return render_modal_workflow(
-                    request, None, 'wagtailimages/chooser/image_chosen.js',
-                    None, json_data={'result': get_image_result_data(image)}
+                    request, None, None,
+                    None, json_data={'step': 'image_chosen', 'result': get_image_result_data(image)}
                 )
     else:
         form = ImageForm(user=request.user)
@@ -166,7 +167,7 @@ def chooser_upload(request):
     paginator, images = paginate(request, images, per_page=12)
 
     return render_modal_workflow(
-        request, 'wagtailimages/chooser/chooser.html', 'wagtailimages/chooser/chooser.js',
+        request, 'wagtailimages/chooser/chooser.html', None,
         {'images': images, 'uploadform': form, 'searchform': searchform},
         json_data=get_chooser_context()
     )
@@ -198,8 +199,8 @@ def chooser_select_format(request, image_id):
             }
 
             return render_modal_workflow(
-                request, None, 'wagtailimages/chooser/image_chosen.js',
-                None, json_data={'result': image_data}
+                request, None, None,
+                None, json_data={'step': 'image_chosen', 'result': image_data}
             )
     else:
         initial = {'alt_text': image.default_alt_text}
@@ -207,6 +208,6 @@ def chooser_select_format(request, image_id):
         form = ImageInsertionForm(initial=initial)
 
     return render_modal_workflow(
-        request, 'wagtailimages/chooser/select_format.html', 'wagtailimages/chooser/select_format.js',
-        {'image': image, 'form': form}
+        request, 'wagtailimages/chooser/select_format.html', None,
+        {'image': image, 'form': form}, json_data={'step': 'select_format'}
     )

wagtail/images/wagtail_hooks.py

@@ -67,7 +67,10 @@ def register_image_feature(features):
         'hallo', 'image',
         HalloPlugin(
             name='hallowagtailimage',
-            js=['wagtailimages/js/hallo-plugins/hallo-wagtailimage.js'],
+            js=[
+                'wagtailimages/js/image-chooser-modal.js',
+                'wagtailimages/js/hallo-plugins/hallo-wagtailimage.js',
+            ],
         )
     )
 
@@ -88,7 +91,9 @@ def register_image_feature(features):
             'whitelist': {
                 'id': True,
             }
-        })
+        }, js=[
+            'wagtailimages/js/image-chooser-modal.js',
+        ])
     )
 
     # define how to convert between contentstate's representation of images and

wagtail/images/widgets.py

@@ -32,4 +32,7 @@ class AdminImageChooser(AdminChooser):
         return "createImageChooser({0});".format(json.dumps(id_))
 
     class Media:
-        js = ['wagtailimages/js/image-chooser.js']
+        js = [
+            'wagtailimages/js/image-chooser-modal.js',
+            'wagtailimages/js/image-chooser.js',
+        ]