From 150e988f4d325c9d2edb693b6ac9b6bba317dc6c Mon Sep 17 00:00:00 2001 From: LB Johnston Date: Thu, 16 Feb 2023 23:33:41 +1000 Subject: [PATCH] Migrate Tagit init JS to TagController - Closes #10100 --- client/src/controllers/TagController.test.js | 86 +++++++++++--- client/src/controllers/TagController.ts | 109 +++++++++++++----- client/src/controllers/index.ts | 2 + client/src/entrypoints/admin/core.js | 3 - client/src/utils/domReady.ts | 18 ++- docs/releases/5.1.md | 33 ++++++ .../wagtailadmin/widgets/tag_widget.html | 7 -- wagtail/admin/tests/test_edit_handlers.py | 9 +- wagtail/admin/tests/test_widgets.py | 69 ++++++----- wagtail/admin/widgets/tags.py | 11 +- .../static_src/wagtaildocs/js/add-multiple.js | 3 - .../bulk_actions/confirm_bulk_add_tags.html | 12 -- .../templates/wagtaildocs/documents/add.html | 3 - .../templates/wagtaildocs/documents/edit.html | 9 -- .../wagtailimages/js/add-multiple.js | 3 - .../bulk_actions/confirm_bulk_add_tags.html | 12 -- .../templates/wagtailimages/images/add.html | 3 - .../templates/wagtailimages/images/edit.html | 9 -- 18 files changed, 248 insertions(+), 153 deletions(-) diff --git a/client/src/controllers/TagController.test.js b/client/src/controllers/TagController.test.js index e5edad4800..9bd44dee5f 100644 --- a/client/src/controllers/TagController.test.js +++ b/client/src/controllers/TagController.test.js @@ -1,33 +1,35 @@ import $ from 'jquery'; +import { Application } from '@hotwired/stimulus'; +import { TagController } from './TagController'; + +const flushPromises = () => new Promise(setImmediate); + window.$ = $; -import { initTagField } from './TagController'; - -describe('initTagField', () => { +describe('TagController', () => { + let application; let element; - const tagitMock = jest.fn(function tagitMockInner() { + const tagitMock = jest.fn(function innerFunction() { element = this; }); window.$.fn.tagit = tagitMock; + element = null; + beforeEach(() => { element = null; jest.clearAllMocks(); }); - it('should not call jQuery tagit if the element is not found', () => { + it('should create a global initTagField to call jQuery tagit if the element is found', async () => { + expect(window.initTagField).toBeUndefined(); expect(tagitMock).not.toHaveBeenCalled(); - initTagField('not-present'); - - expect(tagitMock).not.toHaveBeenCalled(); - }); - - it('should call jQuery tagit if the element is found', () => { - expect(tagitMock).not.toHaveBeenCalled(); + application = Application.start(); + application.register('w-tag', TagController); document.body.innerHTML = `
@@ -35,10 +37,12 @@ describe('initTagField', () => {
`; - initTagField('tag-input', '/path/to/autocomplete/', { + window.initTagField('tag-input', '/path/to/autocomplete/', { someOther: 'option', }); + await flushPromises(); + // check the jQuery instance is the correct element expect(element).toContain(document.getElementById('tag-input')); @@ -59,4 +63,60 @@ describe('initTagField', () => { expect(preprocessTag('"flat white"')).toEqual(`"flat white"`); expect(preprocessTag("'long black'")).toEqual(`"'long black'"`); }); + + it('should not call jQuery tagit if the element is not found', async () => { + expect(tagitMock).not.toHaveBeenCalled(); + + window.initTagField('not-present'); + + await flushPromises(); + + expect(tagitMock).not.toHaveBeenCalled(); + }); + + it('should attach the jQuery tagit to the controlled element', async () => { + document.body.innerHTML = ` +
+ +
`; + + expect(tagitMock).not.toHaveBeenCalled(); + + await flushPromises(); + + expect(tagitMock).toHaveBeenCalledWith({ + allowSpaces: true, + autocomplete: { source: '/admin/tag-autocomplete/' }, + preprocessTag: expect.any(Function), + tagLimit: 10, + }); + + expect(element[0]).toEqual(document.getElementById('id_tags')); + + // check the supplied preprocessTag function + const [{ preprocessTag }] = tagitMock.mock.calls[0]; + + expect(preprocessTag).toBeInstanceOf(Function); + + expect(preprocessTag()).toEqual(); + expect(preprocessTag('"flat white"')).toEqual(`"flat white"`); + expect(preprocessTag("'long black'")).toEqual(`"'long black'"`); + expect(preprocessTag('caffe latte')).toEqual(`"caffe latte"`); + + // check the custom clear behaviour + document + .getElementById('id_tags') + .dispatchEvent(new CustomEvent('example:event')); + + await flushPromises(); + expect(tagitMock).toHaveBeenCalledWith('removeAll'); + }); }); diff --git a/client/src/controllers/TagController.ts b/client/src/controllers/TagController.ts index c714b42432..818df13419 100644 --- a/client/src/controllers/TagController.ts +++ b/client/src/controllers/TagController.ts @@ -1,42 +1,93 @@ import $ from 'jquery'; +import { Controller } from '@hotwired/stimulus'; +import type { Application } from '@hotwired/stimulus'; +import { domReady } from '../utils/domReady'; + declare global { interface JQuery { - tagit(...args): void; + tagit: (options: Record | string) => void; + } + + interface Window { + initTagField: ( + id: string, + url: string, + options?: Record, + ) => void; } } /** - * Initialises the tag fields using the jQuery tagit widget + * Attach the jQuery tagit UI to the controlled element. * - * @param id - element id to initialise against - * @param source - auto complete URL source - * @param options - Other options passed to jQuery tagit + * See https://github.com/aehlke/tag-it + * + * @example + * */ -const initTagField = ( - id: string, - source: string, - options: Record, -): void => { - const tagFieldElement = document.getElementById(id); - - if (!tagFieldElement) return; - - const finalOptions = { - autocomplete: { source }, - preprocessTag(val: any) { - // Double quote a tag if it contains a space - // and if it isn't already quoted. - if (val && val[0] !== '"' && val.indexOf(' ') > -1) { - return '"' + val + '"'; - } - - return val; - }, - ...options, +export class TagController extends Controller { + static values = { + options: { default: {}, type: Object }, + url: String, }; - $('#' + id).tagit(finalOptions); -}; + declare optionsValue: any; + declare urlValue: any; + tagit?: JQuery; -export { initTagField }; + /** + * Prepare a global function that preserves the previous approach to + * registering a tagit field. This will be removed in a future release. + * + * @deprecated RemovedInWagtail60 + */ + static afterLoad( + identifier: string, + { schema: { controllerAttribute } }: Application, + ) { + window.initTagField = (id, url, options = {}): void => { + domReady().then(() => { + const tagFieldElement = document.getElementById(id); + + if (!tagFieldElement || !url) return; + + Object.entries({ options: JSON.stringify(options), url }).forEach( + ([name, value]) => { + tagFieldElement.setAttribute( + `data-${identifier}-${name}-value`, + value, + ); + }, + ); + + tagFieldElement.setAttribute(controllerAttribute, identifier); + }); + }; + } + + connect() { + const preprocessTag = this.cleanTag.bind(this); + + $(this.element).tagit({ + autocomplete: { source: this.urlValue }, + preprocessTag, + ...this.optionsValue, + }); + } + + /** + * Double quote a tag if it contains a space + * and if it isn't already quoted. + */ + cleanTag(val: string) { + return val && val[0] !== '"' && val.indexOf(' ') > -1 ? `"${val}"` : val; + } + + /** + * Method to clear all the tags that are set. + */ + clear() { + $(this.element).tagit('removeAll'); + } +} diff --git a/client/src/controllers/index.ts b/client/src/controllers/index.ts index 4dd8eafa6d..e10ebab0e0 100644 --- a/client/src/controllers/index.ts +++ b/client/src/controllers/index.ts @@ -13,6 +13,7 @@ import { SkipLinkController } from './SkipLinkController'; import { SlugController } from './SlugController'; import { SubmitController } from './SubmitController'; import { SyncController } from './SyncController'; +import { TagController } from './TagController'; import { UpgradeController } from './UpgradeController'; /** @@ -32,5 +33,6 @@ export const coreControllerDefinitions: Definition[] = [ { controllerConstructor: SlugController, identifier: 'w-slug' }, { controllerConstructor: SubmitController, identifier: 'w-submit' }, { controllerConstructor: SyncController, identifier: 'w-sync' }, + { controllerConstructor: TagController, identifier: 'w-tag' }, { controllerConstructor: UpgradeController, identifier: 'w-upgrade' }, ]; diff --git a/client/src/entrypoints/admin/core.js b/client/src/entrypoints/admin/core.js index 6fc323a25e..95faf73590 100644 --- a/client/src/entrypoints/admin/core.js +++ b/client/src/entrypoints/admin/core.js @@ -3,7 +3,6 @@ import $ from 'jquery'; import { coreControllerDefinitions } from '../../controllers'; import { escapeHtml } from '../../utils/text'; import { initStimulus } from '../../includes/initStimulus'; -import { initTagField } from '../../includes/initTagField'; import { initTooltips } from '../../includes/initTooltips'; /** initialise Wagtail Stimulus application with core controller definitions */ @@ -11,8 +10,6 @@ window.Stimulus = initStimulus({ definitions: coreControllerDefinitions }); window.escapeHtml = escapeHtml; -window.initTagField = initTagField; - /* * Enables a "dirty form check", prompting the user if they are navigating away * from a page with unsaved changes, as well as optionally controlling other diff --git a/client/src/utils/domReady.ts b/client/src/utils/domReady.ts index 36edce2633..7ceae77820 100644 --- a/client/src/utils/domReady.ts +++ b/client/src/utils/domReady.ts @@ -1,16 +1,14 @@ /** * Returns a promise that resolves once the DOM is ready for interaction. */ -const domReady = async () => - new Promise((resolve) => { - if (document.readyState === 'loading') { - document.addEventListener('DOMContentLoaded', () => resolve(), { - once: true, - passive: true, - }); - } else { - resolve(); - } +const domReady = async () => { + if (document.readyState !== 'loading') return; + await new Promise((resolve) => { + document.addEventListener('DOMContentLoaded', () => resolve(), { + once: true, + passive: true, + }); }); +}; export { domReady }; diff --git a/docs/releases/5.1.md b/docs/releases/5.1.md index bbdc2ddd05..df913cfe30 100644 --- a/docs/releases/5.1.md +++ b/docs/releases/5.1.md @@ -148,3 +148,36 @@ The ordering for "Object permissions" and "Other permissions" now follows a pred This will be different to the previous ordering which never intentionally implemented. This default ordering is now `["content_type__app_label", "content_type__model", "codename"]`, which can now be customised [](customising_group_views_permissions_order). + +### Tag (Tagit) field usage now relies on data attributes + +The `AdminTagWidget` widget has now been migrated to a Stimulus controller, if using this widget in Python, no changes are needed to adopt the new approach. + +If the widget is being instantiated in JavaScript or HTML with the global util `window.initTagField`, this undocumented util should be replaced with the new `data-*` attributes approach. Additionally, any direct usage of the jQuery widget in JavaScript (e.g. `$('#my-element).tagit()`) should be removed. + +The global util will be removed in a future release. It is recommended that the documented `AdminTagWidget` be used. However, if you need to use the JavaScript approach you can do this with the following example. + +**Old syntax** + +```html + + +``` + +**New syntax** + +```html + +``` + +Note: The `data-w-tag-options-value` is a JSON object serialised into string. Django's HTML escaping will handle it automatically when you use the `AdminTagWidget`, but if you are manually writing the attributes, be sure to use quotation marks correctly. diff --git a/wagtail/admin/templates/wagtailadmin/widgets/tag_widget.html b/wagtail/admin/templates/wagtailadmin/widgets/tag_widget.html index 8be4998f91..4f79b01551 100644 --- a/wagtail/admin/templates/wagtailadmin/widgets/tag_widget.html +++ b/wagtail/admin/templates/wagtailadmin/widgets/tag_widget.html @@ -1,9 +1,2 @@ {% include 'django/forms/widgets/text.html' %}

{{ widget.help_text }}

- \ No newline at end of file diff --git a/wagtail/admin/tests/test_edit_handlers.py b/wagtail/admin/tests/test_edit_handlers.py index bfe8a39651..d17e3c3c7e 100644 --- a/wagtail/admin/tests/test_edit_handlers.py +++ b/wagtail/admin/tests/test_edit_handlers.py @@ -12,7 +12,7 @@ from django.core import checks from django.core.exceptions import FieldDoesNotExist, ImproperlyConfigured from django.test import RequestFactory, TestCase, override_settings from django.urls import reverse -from django.utils.html import json_script +from django.utils.html import escape, json_script from freezegun import freeze_time from wagtail.admin.forms import WagtailAdminModelForm, WagtailAdminPageForm @@ -220,11 +220,14 @@ class TestGetFormForModel(TestCase): fields=["title", "slug", "tags"], ) form_html = RestaurantPageForm().as_p() - self.assertIn("/admin/tag\\u002Dautocomplete/tests/restauranttag/", form_html) + self.assertIn( + 'data-w-tag-url-value="/admin/tag-autocomplete/tests/restauranttag/"', + form_html, + ) # widget should pick up the free_tagging=False attribute on the tag model # and set itself to autocomplete only - self.assertIn('"autocompleteOnly": true', form_html) + self.assertIn(escape('"autocompleteOnly": true'), form_html) # Free tagging should also be disabled at the form field validation level RestaurantTag.objects.create(name="Italian", slug="italian") diff --git a/wagtail/admin/tests/test_widgets.py b/wagtail/admin/tests/test_widgets.py index 9dc26109d2..74930a13cc 100644 --- a/wagtail/admin/tests/test_widgets.py +++ b/wagtail/admin/tests/test_widgets.py @@ -1,8 +1,11 @@ import json +import re +from html import unescape from django import forms from django.test import TestCase from django.test.utils import override_settings +from django.utils.html import escape from wagtail.admin import widgets from wagtail.admin.forms.tags import TagField @@ -387,30 +390,32 @@ class TestAdminDateTimeInput(TestCase): class TestAdminTagWidget(TestCase): def get_js_init_params(self, html): - """Returns a list of the params passed in to initTagField from the supplied HTML""" - # example - ["test_id", "/admin/tag-autocomplete/", {'allowSpaces': True}] - start = "initTagField(" - end = ");" - items_after_init = html.split(start)[1] - if items_after_init: - params_raw = items_after_init.split(end)[0] - if params_raw: - # stuff parameter string into an array so that we can unpack it as JSON - return json.loads("[%s]" % params_raw) - return [] + """ + Returns a list of the key parts of data needed for the w-tag controlled element + An id for the element with the 'w-tag' controller, the autocomplete url & tag options - def get_help_text_html_element(self, html): - """Return a help text html element with content as string""" - start = """""" - end = " -{% endblock %} - {% block header %} {% trans "Add tags to documents" as add_str %} {% include "wagtailadmin/shared/header.html" with title=add_str icon="doc-full-inverse" %} diff --git a/wagtail/documents/templates/wagtaildocs/documents/add.html b/wagtail/documents/templates/wagtaildocs/documents/add.html index 5826df56da..42155d3786 100644 --- a/wagtail/documents/templates/wagtaildocs/documents/add.html +++ b/wagtail/documents/templates/wagtaildocs/documents/add.html @@ -36,9 +36,6 @@ $titleField.val(data.title); } ); - $('#id_tags').tagit({ - autocomplete: {source: "{{ autocomplete_url|addslashes }}"} - }); }); {% endblock %} diff --git a/wagtail/documents/templates/wagtaildocs/documents/edit.html b/wagtail/documents/templates/wagtaildocs/documents/edit.html index 1e89eb32b0..ca80356973 100644 --- a/wagtail/documents/templates/wagtaildocs/documents/edit.html +++ b/wagtail/documents/templates/wagtaildocs/documents/edit.html @@ -7,15 +7,6 @@ {{ block.super }} {{ form.media.js }} - - {% url 'wagtailadmin_tag_autocomplete' as autocomplete_url %} - {% endblock %} {% block extra_css %} diff --git a/wagtail/images/static_src/wagtailimages/js/add-multiple.js b/wagtail/images/static_src/wagtailimages/js/add-multiple.js index 08000e5edc..e809f8c8d6 100644 --- a/wagtail/images/static_src/wagtailimages/js/add-multiple.js +++ b/wagtail/images/static_src/wagtailimages/js/add-multiple.js @@ -214,9 +214,6 @@ $(function () { }); } else { form.replaceWith(data.form); - - // run tagit enhancement on new form - $('.tag_field input', form).tagit(window.tagit_opts); } }); }); diff --git a/wagtail/images/templates/wagtailimages/bulk_actions/confirm_bulk_add_tags.html b/wagtail/images/templates/wagtailimages/bulk_actions/confirm_bulk_add_tags.html index 7a27814134..bb6ce15f3b 100644 --- a/wagtail/images/templates/wagtailimages/bulk_actions/confirm_bulk_add_tags.html +++ b/wagtail/images/templates/wagtailimages/bulk_actions/confirm_bulk_add_tags.html @@ -3,18 +3,6 @@ {% load wagtailimages_tags wagtailadmin_tags %} {% block titletag %}{% blocktrans trimmed count counter=items|length %}Add tags to 1 image {% plural %}Add tags to {{ counter }} images{% endblocktrans %}{% endblock %} -{% block extra_js %} - {{ block.super }} - {% url 'wagtailadmin_tag_autocomplete' as autocomplete_url %} - -{% endblock %} - {% block header %} {% trans "Add tags to images" as add_str %} {% include "wagtailadmin/shared/header.html" with title=add_str icon="doc-full-inverse" %} diff --git a/wagtail/images/templates/wagtailimages/images/add.html b/wagtail/images/templates/wagtailimages/images/add.html index cbfc3716e3..d00d83db6c 100644 --- a/wagtail/images/templates/wagtailimages/images/add.html +++ b/wagtail/images/templates/wagtailimages/images/add.html @@ -36,9 +36,6 @@ $titleField.val(data.title); } ); - $('#id_tags').tagit({ - autocomplete: {source: "{{ autocomplete_url|addslashes }}"} - }); }); {% endblock %} diff --git a/wagtail/images/templates/wagtailimages/images/edit.html b/wagtail/images/templates/wagtailimages/images/edit.html index 71eab940da..d29855acf0 100644 --- a/wagtail/images/templates/wagtailimages/images/edit.html +++ b/wagtail/images/templates/wagtailimages/images/edit.html @@ -12,15 +12,6 @@ {{ form.media.js }} - {% url 'wagtailadmin_tag_autocomplete' as autocomplete_url %} - -