From 2003e14a957d783e1a308765082b8d269c08b16c Mon Sep 17 00:00:00 2001 From: Sage Abdullah Date: Thu, 7 Mar 2024 14:52:28 +0000 Subject: [PATCH] Allow TeleportController to have any number of child nodes of any kind --- .../controllers/TeleportController.test.js | 98 ++++++++++++++++--- client/src/controllers/TeleportController.ts | 20 ++-- 2 files changed, 93 insertions(+), 25 deletions(-) diff --git a/client/src/controllers/TeleportController.test.js b/client/src/controllers/TeleportController.test.js index 0c0700da7414..3daf4a8f4984 100644 --- a/client/src/controllers/TeleportController.test.js +++ b/client/src/controllers/TeleportController.test.js @@ -89,9 +89,9 @@ describe('TeleportController', () => { await Promise.resolve(); - expect(document.getElementById('target-container').innerHTML).toEqual( - '
Some content
', - ); + expect( + document.getElementById('target-container').innerHTML.trim(), + ).toEqual('
Some content
'); }); it('should allow for a default target container within the root element of a shadow DOM', async () => { @@ -136,9 +136,9 @@ describe('TeleportController', () => { await Promise.resolve(); - expect(document.getElementById('target-container').innerHTML).toEqual( - '
Some content
', - ); + expect( + document.getElementById('target-container').innerHTML.trim(), + ).toEqual('
Some content
'); }); it('should not clear the target container if the reset value is unset (false)', async () => { @@ -160,12 +160,84 @@ describe('TeleportController', () => { await Promise.resolve(); + const contents = document.getElementById('target-container').innerHTML; + expect(contents).toContain('

I should still be here

'); + expect(contents).toContain('
Some content
'); + }); + + it('should allow the template to contain multiple children', async () => { + document.body.innerHTML += ` +
+ `; + + const template = document.querySelector('template'); + template.setAttribute( + 'data-w-teleport-target-value', + '#target-container', + ); + + const otherTemplateContent = document.createElement('div'); + otherTemplateContent.innerHTML = 'Other content'; + otherTemplateContent.id = 'other-content'; + template.content.appendChild(otherTemplateContent); + + expect(document.getElementById('target-container').innerHTML).toEqual(''); + + application.start(); + + await Promise.resolve(); + + const container = document.getElementById('target-container'); + const content = container.querySelector('#content'); + const otherContent = container.querySelector('#other-content'); + expect(content).not.toBeNull(); + expect(otherContent).not.toBeNull(); + expect(content.innerHTML.trim()).toEqual('Some content'); + expect(otherContent.innerHTML.trim()).toEqual('Other content'); + }); + + it('should not throw an error if the template content is empty', async () => { + document.body.innerHTML += ` +

I should still be here

+ `; + + const template = document.querySelector('template'); + template.setAttribute( + 'data-w-teleport-target-value', + '#target-container', + ); + + expect(document.getElementById('target-container').innerHTML).toEqual( + '

I should still be here

', + ); + + const errors = []; + + document.getElementById('template').innerHTML = ''; + application.handleError = (error, message) => { + errors.push({ error, message }); + }; + + await Promise.resolve(application.start()); + + expect(errors).toEqual([]); + expect(document.getElementById('target-container').innerHTML).toEqual( - '

I should still be here

Some content
', + '

I should still be here

', ); }); - it('should throw an error if the template content is empty', async () => { + it('should allow erasing the target container by using an empty template with reset value set to true', async () => { + document.body.innerHTML += ` +

I should not be here

+ `; + + const template = document.querySelector('template'); + template.setAttribute( + 'data-w-teleport-target-value', + '#target-container', + ); + template.setAttribute('data-w-teleport-reset-value', 'true'); const errors = []; document.getElementById('template').innerHTML = ''; @@ -176,12 +248,10 @@ describe('TeleportController', () => { await Promise.resolve(application.start()); - expect(errors).toEqual([ - { - error: new Error('Invalid template content.'), - message: 'Error connecting controller', - }, - ]); + expect(errors).toEqual([]); + + const contents = document.getElementById('target-container').innerHTML; + expect(contents).toEqual(''); }); it('should throw an error if a valid target container cannot be resolved', async () => { diff --git a/client/src/controllers/TeleportController.ts b/client/src/controllers/TeleportController.ts index ff9d12602540..3c1221f239cd 100644 --- a/client/src/controllers/TeleportController.ts +++ b/client/src/controllers/TeleportController.ts @@ -46,7 +46,7 @@ export class TeleportController extends Controller { const complete = () => { if (completed) return; if (this.resetValue) target.innerHTML = ''; - target.append(this.templateElement); + target.append(...this.templateFragment.childNodes); this.dispatch('appended', { cancelable: false, detail: { target } }); completed = true; if (this.keepValue) return; @@ -88,22 +88,20 @@ export class TeleportController extends Controller { } /** - * Resolve a valid HTMLElement from the controlled element's children. + * Returns a fresh copy of the DocumentFragment from the controlled element. */ - get templateElement() { - const templateElement = - this.element.content.firstElementChild?.cloneNode(true); - - if (!(templateElement instanceof HTMLElement)) { - throw new Error('Invalid template content.'); - } + get templateFragment() { + const content = this.element.content; + // https://developer.mozilla.org/en-US/docs/Web/API/Node/cloneNode (returns the same type) + // https://github.com/microsoft/TypeScript/issues/283 (TypeScript will return as Node, incorrectly) + const templateFragment = content.cloneNode(true) as typeof content; // HACK: // cloneNode doesn't run scripts, so we need to create new script elements // and copy the attributes and innerHTML over. This is necessary when we're // teleporting a template that contains legacy init code, e.g. initDateChooser. // Only do this for inline scripts, as that's what we're expecting. - templateElement + templateFragment .querySelectorAll('script:not([src], [type])') .forEach((script) => { const newScript = document.createElement('script'); @@ -114,6 +112,6 @@ export class TeleportController extends Controller { script.replaceWith(newScript); }); - return templateElement; + return templateFragment; } }