Convert the page/link chooser to use static onload handlers

.eslintignore

@@ -23,8 +23,4 @@ wagtail/embeds/templates/wagtailembeds/chooser/chooser.js
 wagtail/documents/templates/wagtaildocs/chooser/chooser.js
 wagtail/documents/templates/wagtaildocs/chooser/document_chosen.js
 wagtail/admin/templates/wagtailadmin/page_privacy/set_privacy.js
-wagtail/admin/templates/wagtailadmin/chooser/external_link_chosen.js
-wagtail/admin/templates/wagtailadmin/chooser/external_link.js
-wagtail/admin/templates/wagtailadmin/chooser/email_link.js
-wagtail/admin/templates/wagtailadmin/chooser/browse.js
 wagtail/admin/templates/wagtailadmin/page_privacy/set_privacy_done.js

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

@@ -63,7 +63,7 @@ export const getChooserConfig = (entityType, entity, selectedText) => {
     return {
       url,
       urlParams,
-      onload: {},
+      onload: global.PAGE_CHOOSER_MODAL_ONLOAD_HANDLERS,
     };
 
   case DOCUMENT:

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

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

client/tests/stubs.js

@@ -58,7 +58,9 @@ global.chooserUrls = {
   snippetChooser: '/admin/snippets/choose/',
 };
 
-global.IMAGE_CHOOSER_MODAL_ONLOAD_HANDLERS = {};
+/* use dummy content for onload handlers just so that we can verify that we've chosen the right one */
+global.IMAGE_CHOOSER_MODAL_ONLOAD_HANDLERS = { type: 'image' };
+global.PAGE_CHOOSER_MODAL_ONLOAD_HANDLERS = { type: 'page' };
 
 const jQueryObj = {
   on: jest.fn(),

wagtail/admin/static_src/wagtailadmin/js/hallo-plugins/hallo-wagtaillink.js

@@ -67,6 +67,7 @@
                     return ModalWorkflow({
                         url: url,
                         urlParams: urlParams,
+                        onload: PAGE_CHOOSER_MODAL_ONLOAD_HANDLERS,
                         responses: {
                             pageChosen: function(pageData) {
                                 var a, text, linkHasExistingContent;

wagtail/admin/static_src/wagtailadmin/js/page-chooser-modal.js

@@ -0,0 +1,125 @@
+PAGE_CHOOSER_MODAL_ONLOAD_HANDLERS = {
+    'browse': function(modal, jsonData) {
+        /* Set up link-types links to open in the modal */
+        $('.link-types a', modal.body).on('click', function() {
+            modal.loadUrl(this.href);
+            return false;
+        });
+
+        /*
+        Set up submissions of the search form to open in the modal.
+
+        FIXME: wagtailadmin.views.chooser.browse doesn't actually return a modal-workflow
+        response for search queries, so this just fails with a JS error.
+        Luckily, the search-as-you-type logic below means that we never actually need to
+        submit the form to get search results, so this has the desired effect of preventing
+        plain vanilla form submissions from completing (which would clobber the entire
+        calling page, not just the modal). It would be nice to do that without throwing
+        a JS error, that's all...
+        */
+        modal.ajaxifyForm($('form.search-form', modal.body));
+
+        /* Set up search-as-you-type behaviour on the search box */
+        var searchUrl = $('form.search-form', modal.body).attr('action');
+
+        /* save initial page browser HTML, so that we can restore it if the search box gets cleared */
+        var initialPageResultsHtml = $('.page-results', modal.body).html();
+
+        function search() {
+            var query = $('#id_q', modal.body).val();
+            if (query != '') {
+                $.ajax({
+                    url: searchUrl,
+                    data: {
+                        q: query,
+                        results_only: true
+                    },
+                    success: function(data, status) {
+                        $('.page-results', modal.body).html(data);
+                        ajaxifySearchResults();
+                    }
+                });
+            } else {
+                /* search box is empty - restore original page browser HTML */
+                $('.page-results', modal.body).html(initialPageResultsHtml);
+                ajaxifyBrowseResults();
+            }
+            return false;
+        }
+
+        $('#id_q', modal.body).on('input', function() {
+            clearTimeout($.data(this, 'timer'));
+            var wait = setTimeout(search, 200);
+            $(this).data('timer', wait);
+        });
+
+        /* Set up behaviour of choose-page links in the newly-loaded search results,
+        to pass control back to the calling page */
+        function ajaxifySearchResults() {
+            $('.page-results a.choose-page', modal.body).on('click', function() {
+                var pageData = $(this).data();
+                modal.respond('pageChosen', $(this).data());
+                modal.close();
+
+                return false;
+            });
+            /* pagination links within search results should be AJAX-fetched
+            and the result loaded into .page-results (and ajaxified) */
+            $('.page-results a.navigate-pages', modal.body).on('click', function() {
+                $('.page-results', modal.body).load(this.href, ajaxifySearchResults);
+                return false;
+            });
+        }
+
+        function ajaxifyBrowseResults() {
+            /* Set up page navigation links to open in the modal */
+            $('.page-results a.navigate-pages', modal.body).on('click', function() {
+                modal.loadUrl(this.href);
+                return false;
+            });
+
+            /* Set up behaviour of choose-page links, to pass control back to the calling page */
+            $('a.choose-page', modal.body).on('click', function() {
+                var pageData = $(this).data();
+                pageData.parentId = jsonData['parent_page_id'];
+                modal.respond('pageChosen', $(this).data());
+                modal.close();
+
+                return false;
+            });
+        }
+        ajaxifyBrowseResults();
+
+        /*
+        Focus on the search box when opening the modal.
+        FIXME: this doesn't seem to have any effect (at least on Chrome)
+        */
+        $('#id_q', modal.body).trigger('focus');
+    },
+    'email_link': function(modal, jsonData) {
+        $('p.link-types a', modal.body).on('click', function() {
+            modal.loadUrl(this.href);
+            return false;
+        });
+
+        $('form', modal.body).on('submit', function() {
+            modal.postForm(this.action, $(this).serialize());
+            return false;
+        });
+    },
+    'external_link': function(modal, jsonData) {
+        $('p.link-types a', modal.body).on('click', function() {
+            modal.loadUrl(this.href);
+            return false;
+        });
+
+        $('form', modal.body).on('submit', function() {
+            modal.postForm(this.action, $(this).serialize());
+            return false;
+        });
+    },
+    'external_link_chosen': function(modal, jsonData) {
+        modal.respond('pageChosen', jsonData['result']);
+        modal.close();
+    }
+};

wagtail/admin/static_src/wagtailadmin/js/page-chooser.js

@@ -21,6 +21,7 @@ function createPageChooser(id, pageTypes, openAtParentId, canChooseRoot, userPer
         ModalWorkflow({
             url: initialUrl,
             urlParams: urlParams,
+            onload: PAGE_CHOOSER_MODAL_ONLOAD_HANDLERS,
             responses: {
                 pageChosen: function(pageData) {
                     input.val(pageData.id);

wagtail/admin/templates/wagtailadmin/chooser/browse.js

@@ -1,97 +0,0 @@
-function(modal, jsonData) {
-    /* Set up link-types links to open in the modal */
-    $('.link-types a', modal.body).on('click', function() {
-        modal.loadUrl(this.href);
-        return false;
-    });
-
-    /*
-    Set up submissions of the search form to open in the modal.
-
-    FIXME: wagtailadmin.views.chooser.browse doesn't actually return a modal-workflow
-    response for search queries, so this just fails with a JS error.
-    Luckily, the search-as-you-type logic below means that we never actually need to
-    submit the form to get search results, so this has the desired effect of preventing
-    plain vanilla form submissions from completing (which would clobber the entire
-    calling page, not just the modal). It would be nice to do that without throwing
-    a JS error, that's all...
-    */
-    modal.ajaxifyForm($('form.search-form', modal.body));
-
-    /* Set up search-as-you-type behaviour on the search box */
-    var searchUrl = $('form.search-form', modal.body).attr('action');
-
-    /* save initial page browser HTML, so that we can restore it if the search box gets cleared */
-    var initialPageResultsHtml = $('.page-results', modal.body).html();
-
-    function search() {
-        var query = $('#id_q', modal.body).val();
-        if (query != '') {
-            $.ajax({
-                url: searchUrl,
-                data: {
-                    q: query,
-                    results_only: true
-                },
-                success: function(data, status) {
-                    $('.page-results', modal.body).html(data);
-                    ajaxifySearchResults();
-                }
-            });
-        } else {
-            /* search box is empty - restore original page browser HTML */
-            $('.page-results', modal.body).html(initialPageResultsHtml);
-            ajaxifyBrowseResults();
-        }
-        return false;
-    }
-
-    $('#id_q', modal.body).on('input', function() {
-        clearTimeout($.data(this, 'timer'));
-        var wait = setTimeout(search, 200);
-        $(this).data('timer', wait);
-    });
-
-    /* Set up behaviour of choose-page links in the newly-loaded search results,
-    to pass control back to the calling page */
-    function ajaxifySearchResults() {
-        $('.page-results a.choose-page', modal.body).on('click', function() {
-            var pageData = $(this).data();
-            modal.respond('pageChosen', $(this).data());
-            modal.close();
-
-            return false;
-        });
-        /* pagination links within search results should be AJAX-fetched
-        and the result loaded into .page-results (and ajaxified) */
-        $('.page-results a.navigate-pages', modal.body).on('click', function() {
-            $('.page-results', modal.body).load(this.href, ajaxifySearchResults);
-            return false;
-        });
-    }
-
-    function ajaxifyBrowseResults() {
-        /* Set up page navigation links to open in the modal */
-        $('.page-results a.navigate-pages', modal.body).on('click', function() {
-            modal.loadUrl(this.href);
-            return false;
-        });
-
-        /* Set up behaviour of choose-page links, to pass control back to the calling page */
-        $('a.choose-page', modal.body).on('click', function() {
-            var pageData = $(this).data();
-            pageData.parentId = jsonData['parent_page_id'];
-            modal.respond('pageChosen', $(this).data());
-            modal.close();
-
-            return false;
-        });
-    }
-    ajaxifyBrowseResults();
-
-    /*
-    Focus on the search box when opening the modal.
-    FIXME: this doesn't seem to have any effect (at least on Chrome)
-    */
-    $('#id_q', modal.body).trigger('focus');
-}

wagtail/admin/templates/wagtailadmin/chooser/email_link.js

@@ -1,11 +0,0 @@
-function(modal) {
-    $('p.link-types a', modal.body).on('click', function() {
-        modal.loadUrl(this.href);
-        return false;
-    });
-
-    $('form', modal.body).on('submit', function() {
-        modal.postForm(this.action, $(this).serialize());
-        return false;
-    });
-}

wagtail/admin/templates/wagtailadmin/chooser/external_link.js

@@ -1,11 +0,0 @@
-function(modal) {
-    $('p.link-types a', modal.body).on('click', function() {
-        modal.loadUrl(this.href);
-        return false;
-    });
-
-    $('form', modal.body).on('submit', function() {
-        modal.postForm(this.action, $(this).serialize());
-        return false;
-    });
-}

wagtail/admin/templates/wagtailadmin/chooser/external_link_chosen.js

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

wagtail/admin/tests/test_page_chooser.py

@@ -1,3 +1,5 @@
+import json
+
 from django.contrib.auth import get_user_model
 from django.test import TestCase
 from django.urls import reverse
@@ -467,50 +469,56 @@ class TestChooserExternalLink(TestCase, WagtailTestUtils):
     def test_create_link(self):
         response = self.post({'url': 'http://www.example.com/', 'link_text': 'example'})
         self.assertEqual(response.status_code, 200)
-        self.assertContains(response, '"onload"')  # indicates success / post back to calling page
-        self.assertContains(response, '"url": "http://www.example.com/"')
-        self.assertContains(response, '"title": "example"')  # When link text is given, it is used
-        self.assertContains(response, '"prefer_this_title_as_link_text": true')
+        response_json = json.loads(response.content.decode())
+        self.assertEqual(response_json['step'], 'external_link_chosen')
+        self.assertEqual(response_json['result']['url'], "http://www.example.com/")
+        self.assertEqual(response_json['result']['title'], "example")  # When link text is given, it is used
+        self.assertEqual(response_json['result']['prefer_this_title_as_link_text'], True)
 
     def test_create_link_without_text(self):
         response = self.post({'url': 'http://www.example.com/'})
         self.assertEqual(response.status_code, 200)
-        self.assertContains(response, '"onload"')  # indicates success / post back to calling page
-        self.assertContains(response, '"url": "http://www.example.com/"')
-        self.assertContains(response, '"title": "http://www.example.com/"')  # When no text is given, it uses the url
-        self.assertContains(response, '"prefer_this_title_as_link_text": false')
+        response_json = json.loads(response.content.decode())
+        self.assertEqual(response_json['step'], 'external_link_chosen')
+        self.assertEqual(response_json['result']['url'], "http://www.example.com/")
+        self.assertEqual(response_json['result']['title'], "http://www.example.com/")  # When no text is given, it uses the url
+        self.assertEqual(response_json['result']['prefer_this_title_as_link_text'], False)
 
     def test_notice_changes_to_link_text(self):
         response = self.post(
             {'url': 'http://www.example.com/', 'link_text': 'example'},  # POST data
             {'link_url': 'http://old.example.com/', 'link_text': 'example'}  # GET params - initial data
         )
-        self.assertContains(response, '"url": "http://www.example.com/"')
-        self.assertContains(response, '"title": "example"')
+        result = json.loads(response.content.decode())['result']
+        self.assertEqual(result['url'], "http://www.example.com/")
+        self.assertEqual(result['title'], "example")
         # no change to link text, so prefer the existing link/selection content where available
-        self.assertContains(response, '"prefer_this_title_as_link_text": false')
+        self.assertEqual(result['prefer_this_title_as_link_text'], False)
 
         response = self.post(
             {'url': 'http://www.example.com/', 'link_text': 'new example'},  # POST data
             {'link_url': 'http://old.example.com/', 'link_text': 'example'}  # GET params - initial data
         )
-        self.assertContains(response, '"url": "http://www.example.com/"')
-        self.assertContains(response, '"title": "new example"')
+        result = json.loads(response.content.decode())['result']
+        self.assertEqual(result['url'], "http://www.example.com/")
+        self.assertEqual(result['title'], "new example")
         # link text has changed, so tell the caller to use it
-        self.assertContains(response, '"prefer_this_title_as_link_text": true')
+        self.assertEqual(result['prefer_this_title_as_link_text'], True)
 
     def test_invalid_url(self):
         response = self.post({'url': 'ntp://www.example.com', 'link_text': 'example'})
         self.assertEqual(response.status_code, 200)
-        self.assertContains(response, '"html"')  # indicates failure / show error message
+        response_json = json.loads(response.content.decode())
+        self.assertEqual(response_json['step'], 'external_link')  # indicates failure / show error message
         self.assertContains(response, "Enter a valid URL.")
 
     def test_allow_local_url(self):
         response = self.post({'url': '/admin/', 'link_text': 'admin'})
         self.assertEqual(response.status_code, 200)
-        self.assertContains(response, '"onload"')  # indicates success / post back to calling page
-        self.assertContains(response, '"url": "/admin/"')
-        self.assertContains(response, '"title": "admin"')
+        response_json = json.loads(response.content.decode())
+        self.assertEqual(response_json['step'], 'external_link_chosen')  # indicates success / post back to calling page
+        self.assertEqual(response_json['result']['url'], "/admin/")
+        self.assertEqual(response_json['result']['title'], "admin")
 
 
 class TestChooserEmailLink(TestCase, WagtailTestUtils):
@@ -539,34 +547,38 @@ class TestChooserEmailLink(TestCase, WagtailTestUtils):
 
     def test_create_link(self):
         response = self.post({'email_address': 'example@example.com', 'link_text': 'contact'})
-        self.assertContains(response, '"url": "mailto:example@example.com"')
-        self.assertContains(response, '"title": "contact"')  # When link text is given, it is used
-        self.assertContains(response, '"prefer_this_title_as_link_text": true')
+        result = json.loads(response.content.decode())['result']
+        self.assertEqual(result['url'], "mailto:example@example.com")
+        self.assertEqual(result['title'], "contact")  # When link text is given, it is used
+        self.assertEqual(result['prefer_this_title_as_link_text'], True)
 
     def test_create_link_without_text(self):
         response = self.post({'email_address': 'example@example.com'})
-        self.assertContains(response, '"url": "mailto:example@example.com"')
-        self.assertContains(response, '"title": "example@example.com"')  # When no link text is given, it uses the email
-        self.assertContains(response, '"prefer_this_title_as_link_text": false')
+        result = json.loads(response.content.decode())['result']
+        self.assertEqual(result['url'], "mailto:example@example.com")
+        self.assertEqual(result['title'], "example@example.com")  # When no link text is given, it uses the email
+        self.assertEqual(result['prefer_this_title_as_link_text'], False)
 
     def test_notice_changes_to_link_text(self):
         response = self.post(
             {'email_address': 'example2@example.com', 'link_text': 'example'},  # POST data
             {'link_url': 'example@example.com', 'link_text': 'example'}  # GET params - initial data
         )
-        self.assertContains(response, '"url": "mailto:example2@example.com"')
-        self.assertContains(response, '"title": "example"')
+        result = json.loads(response.content.decode())['result']
+        self.assertEqual(result['url'], "mailto:example2@example.com")
+        self.assertEqual(result['title'], "example")
         # no change to link text, so prefer the existing link/selection content where available
-        self.assertContains(response, '"prefer_this_title_as_link_text": false')
+        self.assertEqual(result['prefer_this_title_as_link_text'], False)
 
         response = self.post(
             {'email_address': 'example2@example.com', 'link_text': 'new example'},  # POST data
             {'link_url': 'example@example.com', 'link_text': 'example'}  # GET params - initial data
         )
-        self.assertContains(response, '"url": "mailto:example2@example.com"')
-        self.assertContains(response, '"title": "new example"')
+        result = json.loads(response.content.decode())['result']
+        self.assertEqual(result['url'], "mailto:example2@example.com")
+        self.assertEqual(result['title'], "new example")
         # link text has changed, so tell the caller to use it
-        self.assertContains(response, '"prefer_this_title_as_link_text": true')
+        self.assertEqual(result['prefer_this_title_as_link_text'], True)
 
 
 class TestCanChoosePage(TestCase, WagtailTestUtils):

wagtail/admin/views/chooser.py

@@ -134,9 +134,9 @@ def browse(request, parent_page_id=None):
 
     return render_modal_workflow(
         request,
-        'wagtailadmin/chooser/browse.html', 'wagtailadmin/chooser/browse.js',
+        'wagtailadmin/chooser/browse.html', None,
         context,
-        json_data={'parent_page_id': context['parent_page_id']},
+        json_data={'step': 'browse', 'parent_page_id': context['parent_page_id']},
     )
 
 
@@ -202,19 +202,18 @@ def external_link(request):
             }
 
             return render_modal_workflow(
-                request,
-                None, 'wagtailadmin/chooser/external_link_chosen.js',
-                None, json_data={'result': result}
+                request, None, None,
+                None, json_data={'step': 'external_link_chosen', 'result': result}
             )
     else:
         form = ExternalLinkChooserForm(initial=initial_data)
 
     return render_modal_workflow(
         request,
-        'wagtailadmin/chooser/external_link.html', 'wagtailadmin/chooser/external_link.js',
+        'wagtailadmin/chooser/external_link.html', None,
         shared_context(request, {
             'form': form,
-        })
+        }), json_data={'step': 'external_link'}
     )
 
 
@@ -237,17 +236,16 @@ def email_link(request):
                 'prefer_this_title_as_link_text': ('link_text' in form.changed_data),
             }
             return render_modal_workflow(
-                request,
-                None, 'wagtailadmin/chooser/external_link_chosen.js',
-                None, json_data={'result': result}
+                request, None, None,
+                None, json_data={'step': 'external_link_chosen', 'result': result}
             )
     else:
         form = EmailLinkChooserForm(initial=initial_data)
 
     return render_modal_workflow(
         request,
-        'wagtailadmin/chooser/email_link.html', 'wagtailadmin/chooser/email_link.js',
+        'wagtailadmin/chooser/email_link.html', None,
         shared_context(request, {
             'form': form,
-        })
+        }), json_data={'step': 'email_link'}
     )

wagtail/admin/wagtail_hooks.py

@@ -271,7 +271,10 @@ def register_core_features(features):
         'hallo', 'link',
         HalloPlugin(
             name='hallowagtaillink',
-            js=['wagtailadmin/js/hallo-plugins/hallo-wagtaillink.js'],
+            js=[
+                'wagtailadmin/js/page-chooser-modal.js',
+                'wagtailadmin/js/hallo-plugins/hallo-wagtaillink.js',
+            ],
         )
     )
     features.register_converter_rule('editorhtml', 'link', [
@@ -502,7 +505,9 @@ def register_core_features(features):
                 # Keep pasted links with http/https protocol, and not-pasted links (href = undefined).
                 'href': "^(http:|https:|undefined$)",
             }
-        })
+        }, js=[
+            'wagtailadmin/js/page-chooser-modal.js',
+        ])
     )
     features.register_converter_rule('contentstate', 'link', {
         'from_database_format': {

wagtail/admin/widgets.py

@@ -213,7 +213,10 @@ class AdminPageChooser(AdminChooser):
         )
 
     class Media:
-        js = ['wagtailadmin/js/page-chooser.js']
+        js = [
+            'wagtailadmin/js/page-chooser-modal.js',
+            'wagtailadmin/js/page-chooser.js',
+        ]
 
 
 @total_ordering