From ae172d42e7d185a8de83cbc3925082c8eac85422 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=A9ry=20Debongnie?= Date: Mon, 6 Apr 2020 15:29:54 +0200 Subject: [PATCH] [IMP] slots: add new t-set-slot directive This new t-set-slot directive is meant to replace t-set when we need to define the content of a sub slot. All new code should use that directive. The old t-set directive is still supported for now, but this should be removed when we publish Owl 2.0. --- doc/reference/slots.md | 9 +- roadmap.md | 2 + src/component/directive.ts | 22 ++++- .../__snapshots__/slots.test.ts.snap | 90 +++++++++++++++++++ tests/component/slots.test.ts | 52 ++++++++--- tools/playground/samples.js | 10 +-- 6 files changed, 164 insertions(+), 21 deletions(-) diff --git a/doc/reference/slots.md b/doc/reference/slots.md index 6f34c2c5..59061fb3 100644 --- a/doc/reference/slots.md +++ b/doc/reference/slots.md @@ -37,16 +37,16 @@ rendering context. ``` -Slots are defined by the caller, with the `t-set` directive: +Slots are defined by the caller, with the `t-set-slot` directive: ```xml
some component
- +
hey
 - + @@ -57,6 +57,9 @@ In this example, the component `Dialog` will render the slots `content` and `foo with its parent as rendering context. This means that clicking on the button will execute the `doSomething` method on the parent, not on the dialog. +Note: Owl previously used the `t-set` directive to define the content of a slot. +This is deprecated and should no longer be used in new code. + ## Reference Default slot: the first element inside the component which is not a named slot will diff --git a/roadmap.md b/roadmap.md index 0d6183e5..d122c610 100644 --- a/roadmap.md +++ b/roadmap.md @@ -16,6 +16,8 @@ change! ### 2.x (2020? 2021? 2022?) +- stop support for `t-set` directive to define the content of a slot + Maybe: - reimplement vdom to use *block* system, like Vue 3, which should make Owl diff --git a/src/component/directive.ts b/src/component/directive.ts index ee1bf14b..8cfbc731 100644 --- a/src/component/directive.ts +++ b/src/component/directive.ts @@ -400,7 +400,16 @@ QWeb.addDirective({ if (hasSlots) { const clone = node.cloneNode(true); - const slotNodes: Element[] = []; + const slotNodes = Array.from(clone.querySelectorAll("[t-set-slot]")); + + // The next code is a fallback for compatibility reason. It accepts t-set + // elements that are direct children with a non empty body as nodes defining + // the content of a slot. + // + // This is wrong, but is necessary to prevent breaking all existing Owl + // code using slots. This will be removed in v2.0 someday. Meanwhile, + // please use t-set-slot everywhere you need to set the content of a + // slot. for (let el of clone.children) { if (el.getAttribute("t-set") && el.hasChildNodes()) { slotNodes.push(el); @@ -412,8 +421,15 @@ QWeb.addDirective({ for (let i = 0, length = slotNodes.length; i < length; i++) { const slotNode = slotNodes[i]; slotNode.parentElement!.removeChild(slotNode); - const key = slotNode.getAttribute("t-set")!; - slotNode.removeAttribute("t-set"); + let key = slotNode.getAttribute("t-set-slot")!; + slotNode.removeAttribute("t-set-slot"); + + // here again, this code should be removed when we stop supporting + // using t-set to define the content of named slots. + if (!key) { + key = slotNode.getAttribute("t-set")!; + slotNode.removeAttribute("t-set"); + } const slotFn = qweb._compile(`slot_${key}_template`, slotNode, ctx); QWeb.slots[`${slotId}_${key}`] = slotFn; } diff --git a/tests/component/__snapshots__/slots.test.ts.snap b/tests/component/__snapshots__/slots.test.ts.snap index 448927e4..ad82605c 100644 --- a/tests/component/__snapshots__/slots.test.ts.snap +++ b/tests/component/__snapshots__/slots.test.ts.snap @@ -90,6 +90,96 @@ exports[`t-slot directive can define and call slots 4`] = ` }" `; +exports[`t-slot directive can define and call slots using old t-set keyword 1`] = ` +"function anonymous(context, extra +) { + // Template name: \\"__template__2\\" + let utils = this.constructor.utils; + let QWeb = this.constructor; + let parent = context; + let scope = Object.create(context); + let h = this.h; + let c1 = [], p1 = {key:1}; + let vn1 = h('div', p1, c1); + // Component 'Dialog' + let w2 = '__3__' in parent.__owl__.cmap ? parent.__owl__.children[parent.__owl__.cmap['__3__']] : false; + let props2 = {}; + if (w2 && w2.__owl__.currentFiber && !w2.__owl__.vnode) { + w2.destroy(); + w2 = false; + } + if (w2) { + w2.__updateProps(props2, extra.fiber, Object.assign(Object.create(context), scope)); + let pvnode = w2.__owl__.pvnode; + c1.push(pvnode); + } else { + let componentKey2 = \`Dialog\`; + let W2 = context.constructor.components[componentKey2] || QWeb.components[componentKey2]|| scope['Dialog']; + if (!W2) {throw new Error('Cannot find the definition of component \\"' + componentKey2 + '\\"')} + w2 = new W2(parent, props2); + parent.__owl__.cmap['__3__'] = w2.__owl__.id; + w2.__owl__.slotId = 1; + let fiber = w2.__prepare(extra.fiber, Object.assign(Object.create(context), scope), () => { const vnode = fiber.vnode; pvnode.sel = vnode.sel; }); + let pvnode = h('dummy', {key: '__3__', hook: {remove() {},destroy(vn) {w2.destroy();}}}); + c1.push(pvnode); + w2.__owl__.pvnode = pvnode; + } + w2.__owl__.parentLastFiberId = extra.fiber.id; + return vn1; +}" +`; + +exports[`t-slot directive can define and call slots using old t-set keyword 2`] = ` +"function anonymous(context, extra +) { + // Template name: \\"__template__1\\" + let h = this.h; + let c6 = [], p6 = {key:6}; + let vn6 = h('div', p6, c6); + let c7 = [], p7 = {key:7}; + let vn7 = h('div', p7, c7); + c6.push(vn7); + const slot8 = this.constructor.slots[context.__owl__.slotId + '_' + 'header']; + if (slot8) { + slot8.call(this, context.__owl__.scope, Object.assign({}, extra, {parentNode: c7, parent: extra.parent || context})); + } + let c9 = [], p9 = {key:9}; + let vn9 = h('div', p9, c9); + c6.push(vn9); + const slot10 = this.constructor.slots[context.__owl__.slotId + '_' + 'footer']; + if (slot10) { + slot10.call(this, context.__owl__.scope, Object.assign({}, extra, {parentNode: c9, parent: extra.parent || context})); + } + return vn6; +}" +`; + +exports[`t-slot directive can define and call slots using old t-set keyword 3`] = ` +"function anonymous(context, extra +) { + // Template name: \\"slot_header_template\\" + let h = this.h; + let c1 = extra.parentNode; + let c4 = [], p4 = {key:4}; + let vn4 = h('span', p4, c4); + c1.push(vn4); + c4.push({text: \`header\`}); +}" +`; + +exports[`t-slot directive can define and call slots using old t-set keyword 4`] = ` +"function anonymous(context, extra +) { + // Template name: \\"slot_footer_template\\" + let h = this.h; + let c1 = extra.parentNode; + let c5 = [], p5 = {key:5}; + let vn5 = h('span', p5, c5); + c1.push(vn5); + c5.push({text: \`footer\`}); +}" +`; + exports[`t-slot directive content is the default slot 1`] = ` "function anonymous(context, extra ) { diff --git a/tests/component/slots.test.ts b/tests/component/slots.test.ts index f43b87bb..7c475807 100644 --- a/tests/component/slots.test.ts +++ b/tests/component/slots.test.ts @@ -41,8 +41,8 @@ describe("t-slot directive", () => {
- header - footer + header + footer
@@ -67,6 +67,38 @@ describe("t-slot directive", () => { expect(QWeb.slots["1_footer"].toString()).toMatchSnapshot(); }); + test("can define and call slots using old t-set keyword", async () => { + // NOTE: this test should be removed once we stop supporting the t-set directive + // for defining slot content. + class Dialog extends Component { + static template = xml` +
+
+
+
`; + } + class Parent extends Component { + static template = xml` +
+ + header + footer + +
`; + static components = { Dialog }; + } + const parent = new Parent(); + await parent.mount(fixture); + + expect(fixture.innerHTML).toBe( + "
header
footer
" + ); + expect(env.qweb.templates[Parent.template].fn.toString()).toMatchSnapshot(); + expect(env.qweb.templates[Dialog.template].fn.toString()).toMatchSnapshot(); + expect(QWeb.slots["1_header"].toString()).toMatchSnapshot(); + expect(QWeb.slots["1_footer"].toString()).toMatchSnapshot(); + }); + test("named slots can define a default content", async () => { class Dialog extends Component { static template = xml` @@ -128,7 +160,7 @@ describe("t-slot directive", () => { `; } class Parent extends Component { - static template = xml`
hey
`; + static template = xml`
hey
`; static components = { Dialog }; } const parent = new Parent(); @@ -143,7 +175,7 @@ describe("t-slot directive", () => {
- +
@@ -302,7 +334,7 @@ describe("t-slot directive", () => {
- +
`; @@ -376,7 +408,7 @@ describe("t-slot directive", () => {
- + sts rocks @@ -442,14 +474,14 @@ describe("t-slot directive", () => { expect(fixture.innerHTML).toBe("
some content
"); }); - test("t-debug on a t-set (defining a slot)", async () => { + test("t-debug on a t-set-slot (defining a slot)", async () => { const consoleLog = console.log; console.log = jest.fn(); env.qweb.addTemplates(`
- abc + abc
@@ -623,8 +655,8 @@ describe("t-slot directive", () => { class A extends Component { static template = xml` - - + + `; static components = { B, C }; } diff --git a/tools/playground/samples.js b/tools/playground/samples.js index e0e3635e..0423f140 100644 --- a/tools/playground/samples.js +++ b/tools/playground/samples.js @@ -1169,7 +1169,7 @@ const RESPONSIVE_CSS = `body { const SLOTS = `// We show here how slots can be used to create generic components. // In this example, the Card component is basically only a container. It is not // aware of its content. It just knows where it should be (with t-slot). -// The parent component define the content with t-set. +// The parent component define the content with t-set-slot. // // Note that the t-on-click event, defined in the App template, is executed in // the context of the App component, even though it is inside the Card component @@ -1235,15 +1235,15 @@ const SLOTS_XML = `
- Content of card 1... [] - + Content of card 1... [] + - +
Card 2... []
 - +
`;