diff --git a/CHANGELOG.txt b/CHANGELOG.txt index a8940ef37e..bd66b04ae8 100644 --- a/CHANGELOG.txt +++ b/CHANGELOG.txt @@ -111,6 +111,7 @@ Changelog * Maintenance: Fix search tests to correctly reflect behaviour of search backends other than the fallback backend (Matt Westcott) * Maintenance: Migrate select all checkbox in simple translation's submit translation page to Stimulus controller `w-bulk`, remove inline script usage (Hanoon) * Maintenance: Refactor `SnippetViewSet` to extend `ModelViewSet` (Sage Abdullah) + * Maintenance: Migrate initDismissibles behaviour to a Stimulus controller `w-disimissible` (Loveth Omokaro) 4.2.2 (03.04.2023) diff --git a/client/src/components/Sidebar/modules/MainMenu.tsx b/client/src/components/Sidebar/modules/MainMenu.tsx index e2e4327602..cec4eb32eb 100644 --- a/client/src/components/Sidebar/modules/MainMenu.tsx +++ b/client/src/components/Sidebar/modules/MainMenu.tsx @@ -8,7 +8,7 @@ import { LinkMenuItemDefinition } from '../menu/LinkMenuItem'; import { MenuItemDefinition } from '../menu/MenuItem'; import { SubMenuItemDefinition } from '../menu/SubMenuItem'; import { ModuleDefinition } from '../Sidebar'; -import { updateDismissibles } from '../../../includes/initDismissibles'; +import { updateDismissibles } from '../../../controllers/DismissibleController'; export function renderMenu( path: string, @@ -36,9 +36,9 @@ export function renderMenu( export function isDismissed(item: MenuItemDefinition, state: MenuState) { return ( // Non-dismissibles are considered as dismissed - !item.attrs['data-wagtail-dismissible-id'] || + !item.attrs['data-w-dismissible-id-value'] || // Dismissed on the server - 'data-wagtail-dismissed' in item.attrs || + 'data-w-dismissible-dismissed-value' in item.attrs || // Dismissed on the client state.dismissibles[item.name] ); @@ -76,7 +76,7 @@ function walkDismissibleMenuItems( action: (item: MenuItemDefinition) => void, ) { menuItems.forEach((menuItem) => { - const id = menuItem.attrs['data-wagtail-dismissible-id']; + const id = menuItem.attrs['data-w-dismissible-id-value']; if (id) { action(menuItem); } @@ -95,7 +95,7 @@ function computeDismissibleState( // Recursively update all dismissible items walkDismissibleMenuItems([item], (menuItem) => { - update[menuItem.attrs['data-wagtail-dismissible-id']] = value; + update[menuItem.attrs['data-w-dismissible-id-value']] = value; }); // Send the update to the server @@ -132,8 +132,8 @@ function getInitialDismissibleState(menuItems: MenuItemDefinition[]) { const result: Record = {}; walkDismissibleMenuItems(menuItems, (menuItem) => { - result[menuItem.attrs['data-wagtail-dismissible-id']] = - 'data-wagtail-dismissed' in menuItem.attrs; + result[menuItem.attrs['data-w-dismissible-id-value']] = + 'data-w-dismissible-dismissed-value' in menuItem.attrs; }); return result; diff --git a/client/src/controllers/DismissibleController.test.js b/client/src/controllers/DismissibleController.test.js index 84808e7c08..9b97f8b0f6 100644 --- a/client/src/controllers/DismissibleController.test.js +++ b/client/src/controllers/DismissibleController.test.js @@ -1,4 +1,8 @@ -import { initDismissibles, updateDismissibles } from './initDismissibles'; +import { Application } from '@hotwired/stimulus'; +import { + DismissibleController, + updateDismissibles, +} from './DismissibleController'; jest.mock('../config/wagtailConfig.js', () => ({ WAGTAIL_CONFIG: { @@ -8,50 +12,62 @@ jest.mock('../config/wagtailConfig.js', () => ({ }, })); -describe('initDismissibles', () => { - it('should not error if there are no dismissibles', () => { - document.body.innerHTML = '
CONTENT
'; +describe('DismissibleController', () => { + let application; + const data = { whats_new_in_wagtail_version_4: true }; - initDismissibles(); + beforeEach(() => { + application?.stop(); - expect(document.body.innerHTML).toContain('CONTENT'); + document.body.innerHTML = ` +
+ +
+ `; + + application = Application.start(); + application.register('w-dismissible', DismissibleController); }); - describe('should initialise dismissibles', () => { - it('for data-wagtail-dismissible-id and data-wagtail-dismissible-toggle attribute in parent and child', () => { - document.body.innerHTML = ` -
-
-
-
-
-
-
-
{ + const button = document.querySelector('button'); + const mainContent = document.querySelector('#main-content'); - initDismissibles(); + expect(mainContent.classList).toHaveLength(0); + expect( + mainContent.getAttribute('data-w-dismissible-dismissed-value'), + ).toBeFalsy(); + expect(mainContent.classList).not.toContain('w-dismissible--dismissed'); - // check the classes are initially empty - expect( - document.querySelector('[data-wagtail-dismissible-id]').classList, - ).toHaveLength(0); + button.click(); - // click all buttons - document - .querySelectorAll('[data-wagtail-dismissible-toggle]') - .forEach((item) => { - item.click(); - }); + expect(mainContent.classList).toContain('w-dismissible--dismissed'); + expect(mainContent.getAttribute('data-w-dismissible-dismissed-value')).toBe( + 'true', + ); + }); - // check the classes are updated and data attribute removed - expect( - [...document.querySelectorAll('[data-wagtail-dismissible-id]')].every( - (item) => - item.classList.contains('w-dismissible--dismissed') && - item.getAttribute('data-wagtail-dismissed') === '', - ), - ).toBe(true); + it('should update the dismissible ids when the dismiss button is clicked', async () => { + expect.assertions(1); + + const button = document.querySelector('button'); + + button.click(); + + await expect(global.fetch).toHaveBeenCalledWith('/admin/dismissibles/', { + method: 'PATCH', + headers: { + 'Content-Type': 'application/json', + 'X-CSRFToken': 'test-token', + }, + body: JSON.stringify(data), + mode: 'same-origin', }); }); }); diff --git a/client/src/controllers/DismissibleController.ts b/client/src/controllers/DismissibleController.ts index 6932499344..7f16c94053 100644 --- a/client/src/controllers/DismissibleController.ts +++ b/client/src/controllers/DismissibleController.ts @@ -1,65 +1,68 @@ +import { Controller } from '@hotwired/stimulus'; import { WAGTAIL_CONFIG } from '../config/wagtailConfig'; /** * Updates the server, using a PATCH request when the toggle is clicked on a dismissible - * element initialised by initDismissibles + * element initialised by DismissibleController * * @param data - The dismissible represented as an object with keys as * the id and its new state: whether it is dismissed (boolean) * - * @return {Promise} - * * @example * const data = { 'dismissible-1': true, 'dismissible-2': false }; * const wagtailConfig = {} * * updateDismissibles(data, wagtailConfig); */ -export function updateDismissibles( +export const updateDismissibles = ( data: Record, -): Promise { - return fetch(WAGTAIL_CONFIG.ADMIN_URLS?.DISMISSIBLES, { +): Promise => + fetch(WAGTAIL_CONFIG.ADMIN_URLS?.DISMISSIBLES, { method: 'PATCH', headers: { [WAGTAIL_CONFIG.CSRF_HEADER_NAME]: WAGTAIL_CONFIG.CSRF_TOKEN, - 'Content-Type': 'application/json', + 'Content-Type': 'application/json', // eslint-disable-line @typescript-eslint/naming-convention }, body: JSON.stringify(data), mode: 'same-origin', }); -} /** - * Initialise dismissibles fetched from server and add click event listeners to them. - * @return {void} + * Adds the ability to make an element dismissible so that it updates it's class and makes an async request. + * Initialise such elements with a default handler that performs the dismissal. + * This only initialises elements that are rendered by the server (if they have the data attr), so elements + * that are rendered by the client (e.g. React) needs to be handled separately. + * + * @example + *
+ * + *
*/ -export function initDismissibles(): void { - // A dismissible element is marked by the data-wagtail-dismissible-id attribute. - const dismissibles = document.querySelectorAll( - '[data-wagtail-dismissible-id]', - ); +export class DismissibleController extends Controller { + static classes = ['dismissed']; - // Initialise such elements with a default handler that performs the dismissal. - // This only initialises elements that are rendered by the server, so elements - // that are rendered by the client (e.g. React) needs to be handled separately. - dismissibles.forEach((dismissible: HTMLElement) => { - // The toggle is marked by the data-wagtail-dismissible-toggle attribute, - // which can either be the dismissible itself or a descendant element. - const toggle = dismissible.hasAttribute('data-wagtail-dismissible-toggle') - ? dismissible - : dismissible.querySelector( - '[data-wagtail-dismissible-toggle]', - ); - const id = dismissible.dataset.wagtailDismissibleId; - if (!(toggle && id)) return; + static values = { + dismissed: { default: false, type: Boolean }, + id: { default: '', type: String }, + }; - // Upon clicking the toggle, send an update to the server and add the - // appropriate class and data attribute optimistically. Each dismissible - // defines how it uses (or not) these indicators. - toggle.addEventListener('click', () => { - updateDismissibles({ [id]: true }); - dismissible.classList.add('w-dismissible--dismissed'); - dismissible.setAttribute('data-wagtail-dismissed', ''); - }); - }); + declare dismissedValue: boolean; + declare idValue: string; + declare readonly dismissedClass: string; + + /** + * Upon activating the toggle, send an update to the server and add the + * appropriate class and data attribute optimistically. Each dismissible + * defines how it uses (or not) these indicators. + */ + toggle(): void { + if (!this.idValue) return; + this.element.classList.add(this.dismissedClass); + this.dismissedValue = true; + updateDismissibles({ [this.idValue]: true }); + } } diff --git a/client/src/controllers/index.ts b/client/src/controllers/index.ts index 12abc50745..1e16f1c95e 100644 --- a/client/src/controllers/index.ts +++ b/client/src/controllers/index.ts @@ -4,6 +4,7 @@ import type { Definition } from '@hotwired/stimulus'; import { ActionController } from './ActionController'; import { BulkController } from './BulkController'; import { CountController } from './CountController'; +import { DismissibleController } from './DismissibleController'; import { MessagesController } from './MessagesController'; import { ProgressController } from './ProgressController'; import { SkipLinkController } from './SkipLinkController'; @@ -20,6 +21,7 @@ export const coreControllerDefinitions: Definition[] = [ { controllerConstructor: ActionController, identifier: 'w-action' }, { controllerConstructor: BulkController, identifier: 'w-bulk' }, { controllerConstructor: CountController, identifier: 'w-count' }, + { controllerConstructor: DismissibleController, identifier: 'w-dismissible' }, { controllerConstructor: MessagesController, identifier: 'w-messages' }, { controllerConstructor: ProgressController, identifier: 'w-progress' }, { controllerConstructor: SkipLinkController, identifier: 'w-skip-link' }, diff --git a/client/src/entrypoints/admin/wagtailadmin.js b/client/src/entrypoints/admin/wagtailadmin.js index 019aa9cbfe..a5dc72c6d0 100644 --- a/client/src/entrypoints/admin/wagtailadmin.js +++ b/client/src/entrypoints/admin/wagtailadmin.js @@ -1,4 +1,4 @@ -import { Icon, Portal, initDismissibles } from '../..'; +import { Icon, Portal } from '../..'; import { initModernDropdown, initTooltips } from '../../includes/initTooltips'; import { initTabs } from '../../includes/tabs'; import { dialog } from '../../includes/dialog'; @@ -23,7 +23,6 @@ document.addEventListener('DOMContentLoaded', () => { initTooltips(); initModernDropdown(); initTabs(); - initDismissibles(); dialog(); initCollapsibleBreadcrumbs(); initSidePanel(); diff --git a/client/src/index.ts b/client/src/index.ts index a298622861..dcff94697a 100644 --- a/client/src/index.ts +++ b/client/src/index.ts @@ -9,4 +9,3 @@ export { default as LoadingSpinner } from './components/LoadingSpinner/LoadingSp export { default as Portal } from './components/Portal/Portal'; export { default as PublicationStatus } from './components/PublicationStatus/PublicationStatus'; export { default as Transition } from './components/Transition/Transition'; -export { initDismissibles } from './includes/initDismissibles'; diff --git a/docs/releases/5.0.md b/docs/releases/5.0.md index 20b2eefeb0..c283c26b23 100644 --- a/docs/releases/5.0.md +++ b/docs/releases/5.0.md @@ -158,6 +158,7 @@ Those improvements were implemented by Albina Starykova as part of an [Outreachy * Fix search tests to correctly reflect behaviour of search backends other than the fallback backend (Matt Westcott) * Migrate select all checkbox in simple translation's submit translation page to Stimulus controller `w-bulk`, remove inline script usage (Hanoon) * Refactor `SnippetViewSet` to extend `ModelViewSet` (Sage Abdullah) + * Migrate initDismissibles behaviour to a Stimulus controller `w-disimissible` (Loveth Omokaro) ## Upgrade considerations diff --git a/wagtail/admin/menu.py b/wagtail/admin/menu.py index 4870f7430d..1be125f2d1 100644 --- a/wagtail/admin/menu.py +++ b/wagtail/admin/menu.py @@ -44,17 +44,19 @@ class MenuItem(metaclass=MediaDefiningClass): class DismissibleMenuItemMixin: def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) - self.attrs["data-wagtail-dismissible-id"] = self.name + self.attrs["data-controller"] = "w-dismissible" + self.attrs["data-w-dismissible-dismissed-class"] = "w-dismissible--dismissed" + self.attrs["data-w-dismissible-id-value"] = self.name def render_component(self, request): profile = getattr(request.user, "wagtail_userprofile", None) # Menu item instances are cached, so make sure the existence of the - # data-wagtail-dismissed attribute is correct for the user + # data-w-dismissible-dismissed-value attribute is correct for the user if profile and profile.dismissibles.get(self.name): - self.attrs["data-wagtail-dismissed"] = "" + self.attrs["data-w-dismissible-dismissed-value"] = "true" else: - self.attrs.pop("data-wagtail-dismissed", None) + self.attrs.pop("data-w-dismissible-dismissed-value", None) return super().render_component(request) diff --git a/wagtail/admin/templates/wagtailadmin/home/whats_new_in_wagtail_version.html b/wagtail/admin/templates/wagtailadmin/home/whats_new_in_wagtail_version.html index 78b16a0291..5f8858241f 100644 --- a/wagtail/admin/templates/wagtailadmin/home/whats_new_in_wagtail_version.html +++ b/wagtail/admin/templates/wagtailadmin/home/whats_new_in_wagtail_version.html @@ -1,8 +1,14 @@ {% load i18n wagtailadmin_tags wagtailcore_tags %} -
+
- diff --git a/wagtail/admin/tests/test_menu.py b/wagtail/admin/tests/test_menu.py index 0797b70858..856325fa23 100644 --- a/wagtail/admin/tests/test_menu.py +++ b/wagtail/admin/tests/test_menu.py @@ -153,8 +153,13 @@ class TestMenuRendering(WagtailTestUtils, TestCase): self.assertEqual( rendered[0].attrs, # Should not be dismissed - {"data-wagtail-dismissible-id": "dismissible-submenu-menu-item"}, + { + "data-controller": "w-dismissible", + "data-w-dismissible-dismissed-class": "w-dismissible--dismissed", + "data-w-dismissible-id-value": "dismissible-submenu-menu-item", + }, ) + self.assertListEqual( rendered[0].menu_items, [ @@ -163,7 +168,11 @@ class TestMenuRendering(WagtailTestUtils, TestCase): "Pages", "/pages/", # Should not be dismissed - attrs={"data-wagtail-dismissible-id": "dismissible-menu-item"}, + attrs={ + "data-controller": "w-dismissible", + "data-w-dismissible-dismissed-class": "w-dismissible--dismissed", + "data-w-dismissible-id-value": "dismissible-menu-item", + }, ), ], ) @@ -211,9 +220,11 @@ class TestMenuRendering(WagtailTestUtils, TestCase): self.assertEqual( rendered[0].attrs, { - "data-wagtail-dismissible-id": "dismissible-submenu-menu-item", + "data-controller": "w-dismissible", + "data-w-dismissible-dismissed-class": "w-dismissible--dismissed", + "data-w-dismissible-id-value": "dismissible-submenu-menu-item", # Should be dismissed - "data-wagtail-dismissed": "", + "data-w-dismissible-dismissed-value": "true", }, ) self.assertListEqual( @@ -225,8 +236,10 @@ class TestMenuRendering(WagtailTestUtils, TestCase): "/pages/", # Should be dismissed attrs={ - "data-wagtail-dismissible-id": "dismissible-menu-item", - "data-wagtail-dismissed": "", + "data-controller": "w-dismissible", + "data-w-dismissible-dismissed-class": "w-dismissible--dismissed", + "data-w-dismissible-id-value": "dismissible-menu-item", + "data-w-dismissible-dismissed-value": "true", }, ), ], @@ -271,7 +284,11 @@ class TestMenuRendering(WagtailTestUtils, TestCase): self.assertEqual(rendered[0].label, "My dismissible submenu") self.assertEqual( rendered[0].attrs, - {"data-wagtail-dismissible-id": "dismissible-submenu-menu-item"}, + { + "data-controller": "w-dismissible", + "data-w-dismissible-dismissed-class": "w-dismissible--dismissed", + "data-w-dismissible-id-value": "dismissible-submenu-menu-item", + }, ) self.assertListEqual( rendered[0].menu_items, @@ -280,7 +297,11 @@ class TestMenuRendering(WagtailTestUtils, TestCase): "dismissible-menu-item", "Pages", "/pages/", - attrs={"data-wagtail-dismissible-id": "dismissible-menu-item"}, + attrs={ + "data-controller": "w-dismissible", + "data-w-dismissible-dismissed-class": "w-dismissible--dismissed", + "data-w-dismissible-id-value": "dismissible-menu-item", + }, ), ], ) diff --git a/wagtail/admin/tests/test_whats_new.py b/wagtail/admin/tests/test_whats_new.py index 69b00e9554..2421a049b1 100644 --- a/wagtail/admin/tests/test_whats_new.py +++ b/wagtail/admin/tests/test_whats_new.py @@ -29,10 +29,13 @@ class TestWhatsNewInWagtailVersionPanel(WagtailTestUtils, TestCase): def test_render_html_user_initial(self): result = self.panel.render_html(self.get_parent_context()) - self.assertIn( - f'
', - result, - ) + expected_data_attrs = [ + 'data-controller="w-dismissible"', + 'data-w-dismissible-dismissed-class="w-dismissible--dismissed"', + f'data-w-dismissible-id-value="{self.dismissible_id}"', + ] + for data_attr in expected_data_attrs: + self.assertIn(data_attr, result) self.assertIn("Things in Wagtail 4 have changed!", result) @override_settings(WAGTAIL_ENABLE_WHATS_NEW_BANNER=False) @@ -44,10 +47,13 @@ class TestWhatsNewInWagtailVersionPanel(WagtailTestUtils, TestCase): self.profile.delete() self.user.refresh_from_db() result = self.panel.render_html(self.get_parent_context()) - self.assertIn( - f'
', - result, - ) + expected_data_attrs = [ + 'data-controller="w-dismissible"', + 'data-w-dismissible-dismissed-class="w-dismissible--dismissed"', + f'data-w-dismissible-id-value="{self.dismissible_id}"', + ] + for data_attr in expected_data_attrs: + self.assertIn(data_attr, result) self.assertIn("Things in Wagtail 4 have changed!", result) def test_render_html_user_dismissed(self): @@ -70,29 +76,41 @@ class TestWhatsNewOnDashboard(WagtailTestUtils, TestCase): def test_get_enabled_initial(self): response = self.get() - self.assertContains( - response, - f'
', - ) + html_content = response.content.decode("utf-8") + expected_data_attrs = [ + 'data-controller="w-dismissible"', + 'data-w-dismissible-dismissed-class="w-dismissible--dismissed"', + f'data-w-dismissible-id-value="{self.dismissible_id}"', + ] + for data_attr in expected_data_attrs: + self.assertIn(data_attr, html_content) self.assertContains(response, "Things in Wagtail 4 have changed!") @override_settings(WAGTAIL_ENABLE_WHATS_NEW_BANNER=False) def test_get_disabled_initial(self): response = self.get() - self.assertNotContains( - response, - f'
', - ) + html_content = response.content.decode("utf-8") + expected_data_attrs = [ + 'data-controller="w-dismissible"', + 'data-w-dismissible-dismissed-class="w-dismissible--dismissed"', + f'data-w-dismissible-id-value="{self.dismissible_id}"', + ] + for data_attr in expected_data_attrs: + self.assertNotIn(data_attr, html_content) self.assertNotContains(response, "Things in Wagtail 4 have changed!") def test_render_html_user_no_profile(self): self.profile.delete() self.user.refresh_from_db() response = self.get() - self.assertContains( - response, - f'
', - ) + html_content = response.content.decode("utf-8") + expected_data_attrs = [ + 'data-controller="w-dismissible"', + 'data-w-dismissible-dismissed-class="w-dismissible--dismissed"', + f'data-w-dismissible-id-value="{self.dismissible_id}"', + ] + for data_attr in expected_data_attrs: + self.assertIn(data_attr, html_content) self.assertContains(response, "Things in Wagtail 4 have changed!") def test_get_enabled_dismissed(self): @@ -100,8 +118,12 @@ class TestWhatsNewOnDashboard(WagtailTestUtils, TestCase): self.profile.save(update_fields=["dismissibles"]) response = self.get() - self.assertNotContains( - response, - f'
', - ) + html_content = response.content.decode("utf-8") + expected_data_attrs = [ + 'data-controller="w-dismissible"', + 'data-w-dismissible-dismissed-class="w-dismissible--dismissed"', + f'data-w-dismissible-id-value="{self.dismissible_id}"', + ] + for data_attr in expected_data_attrs: + self.assertNotIn(data_attr, html_content) self.assertNotContains(response, "Things in Wagtail 4 have changed!")