[LiveComponent] Fix loadingDirectives match elements from nested components

smnandre · weaverryan · commit 7867e44ccc0e · 2024-01-30T10:36:06.000-05:00
diff --git a/src/LiveComponent/assets/dist/Component/plugins/LoadingPlugin.d.ts b/src/LiveComponent/assets/dist/Component/plugins/LoadingPlugin.d.ts
@@ -8,11 +8,11 @@ interface ElementLoadingDirectives {
 }
 export default class implements PluginInterface {
     attachToComponent(component: Component): void;
-    startLoading(targetElement: HTMLElement | SVGElement, backendRequest: BackendRequest): void;
-    finishLoading(targetElement: HTMLElement | SVGElement): void;
+    startLoading(component: Component, targetElement: HTMLElement | SVGElement, backendRequest: BackendRequest): void;
+    finishLoading(component: Component, targetElement: HTMLElement | SVGElement): void;
     private handleLoadingToggle;
     private handleLoadingDirective;
-    getLoadingDirectives(element: HTMLElement | SVGElement): ElementLoadingDirectives[];
+    getLoadingDirectives(component: Component, element: HTMLElement | SVGElement): ElementLoadingDirectives[];
     private showElement;
     private hideElement;
     private addClass;
diff --git a/src/LiveComponent/assets/dist/live_controller.js b/src/LiveComponent/assets/dist/live_controller.js
@@ -2370,27 +2370,27 @@ class StandardElementDriver {
 class LoadingPlugin {
     attachToComponent(component) {
         component.on('loading.state:started', (element, request) => {
-            this.startLoading(element, request);
+            this.startLoading(component, element, request);
         });
         component.on('loading.state:finished', (element) => {
-            this.finishLoading(element);
+            this.finishLoading(component, element);
         });
-        this.finishLoading(component.element);
+        this.finishLoading(component, component.element);
     }
-    startLoading(targetElement, backendRequest) {
-        this.handleLoadingToggle(true, targetElement, backendRequest);
+    startLoading(component, targetElement, backendRequest) {
+        this.handleLoadingToggle(component, true, targetElement, backendRequest);
     }
-    finishLoading(targetElement) {
-        this.handleLoadingToggle(false, targetElement, null);
+    finishLoading(component, targetElement) {
+        this.handleLoadingToggle(component, false, targetElement, null);
     }
-    handleLoadingToggle(isLoading, targetElement, backendRequest) {
+    handleLoadingToggle(component, isLoading, targetElement, backendRequest) {
         if (isLoading) {
             this.addAttributes(targetElement, ['busy']);
         }
         else {
             this.removeAttributes(targetElement, ['busy']);
         }
-        this.getLoadingDirectives(targetElement).forEach(({ element, directives }) => {
+        this.getLoadingDirectives(component, targetElement).forEach(({ element, directives }) => {
             if (isLoading) {
                 this.addAttributes(element, ['data-live-is-loading']);
             }
@@ -2474,9 +2474,10 @@ class LoadingPlugin {
         }
         loadingDirective();
     }
-    getLoadingDirectives(element) {
+    getLoadingDirectives(component, element) {
         const loadingDirectives = [];
-        let matchingElements = element.querySelectorAll('[data-loading]');
+        let matchingElements = [...element.querySelectorAll('[data-loading]')];
+        matchingElements = matchingElements.filter((elt) => elementBelongsToThisComponent(elt, component));
         if (element.hasAttribute('data-loading')) {
             matchingElements = [element, ...matchingElements];
         }
diff --git a/src/LiveComponent/assets/src/Component/plugins/LoadingPlugin.ts b/src/LiveComponent/assets/src/Component/plugins/LoadingPlugin.ts
@@ -3,6 +3,7 @@ import {
     DirectiveModifier,
     parseDirectives
 } from '../../Directive/directives_parser';
+import { elementBelongsToThisComponent } from '../../dom_utils';
 import { combineSpacedArray } from '../../string_utils';
 import BackendRequest from '../../Backend/BackendRequest';
 import Component from '../../Component';
@@ -16,32 +17,32 @@ interface ElementLoadingDirectives {
 export default class implements PluginInterface {
     attachToComponent(component: Component): void {
         component.on('loading.state:started', (element: HTMLElement, request: BackendRequest) => {
-            this.startLoading(element, request);
+            this.startLoading(component, element, request);
         });
         component.on('loading.state:finished', (element: HTMLElement) => {
-            this.finishLoading(element);
+            this.finishLoading(component, element);
         });
         // hide "loading" elements to begin with
         // This is done with CSS, but only for the most basic cases
-        this.finishLoading(component.element);
+        this.finishLoading(component, component.element);
     }
 
-    startLoading(targetElement: HTMLElement|SVGElement, backendRequest: BackendRequest): void {
-        this.handleLoadingToggle(true, targetElement, backendRequest);
+    startLoading(component: Component, targetElement: HTMLElement|SVGElement, backendRequest: BackendRequest): void {
+        this.handleLoadingToggle(component, true, targetElement, backendRequest);
     }
 
-    finishLoading(targetElement: HTMLElement|SVGElement): void {
-        this.handleLoadingToggle(false, targetElement, null);
+    finishLoading(component: Component, targetElement: HTMLElement|SVGElement): void {
+        this.handleLoadingToggle(component,false, targetElement, null);
     }
 
-    private handleLoadingToggle(isLoading: boolean, targetElement: HTMLElement|SVGElement, backendRequest: BackendRequest|null) {
+    private handleLoadingToggle(component: Component, isLoading: boolean, targetElement: HTMLElement|SVGElement, backendRequest: BackendRequest|null) {
         if (isLoading) {
             this.addAttributes(targetElement, ['busy']);
         } else {
             this.removeAttributes(targetElement, ['busy']);
         }
 
-        this.getLoadingDirectives(targetElement).forEach(({ element, directives }) => {
+        this.getLoadingDirectives(component, targetElement).forEach(({ element, directives }) => {
             // so we can track, at any point, if an element is in a "loading" state
             if (isLoading) {
                 this.addAttributes(element, ['data-live-is-loading']);
@@ -148,9 +149,12 @@ export default class implements PluginInterface {
         loadingDirective();
     }
 
-    getLoadingDirectives(element: HTMLElement|SVGElement) {
+    getLoadingDirectives(component: Component, element: HTMLElement|SVGElement) {
         const loadingDirectives: ElementLoadingDirectives[] = [];
-        let matchingElements = element.querySelectorAll('[data-loading]');
+        let matchingElements = [...element.querySelectorAll('[data-loading]')];
+
+        // ignore elements which are inside a nested "live" component
+        matchingElements = matchingElements.filter((elt) => elementBelongsToThisComponent(elt, component));
 
         // querySelectorAll doesn't include the element itself
         if (element.hasAttribute('data-loading')) {
diff --git a/src/LiveComponent/assets/test/controller/loading.test.ts b/src/LiveComponent/assets/test/controller/loading.test.ts
@@ -223,4 +223,51 @@ describe('LiveController data-loading Tests', () => {
         await (new Promise(resolve => setTimeout(resolve, 30)));
         expect(getByTestId(test.element, 'loading-element')).not.toBeVisible();
     });
+
+    it('does not trigger loading inside component children', async () => {
+       const childTemplate = (data: any) => `
+            <div ${initComponent(data, {id: 'child-id'})} data-testid="child">
+                <span data-loading="show" data-testid="child-loading-element-showing">Loading...</span>
+                <span data-loading="hide" data-testid="child-loading-element-hiding">Loading...</span>
+            </div>
+        `;
+
+        const test = await createTest({} , (data: any) => `
+            <div ${initComponent(data, {id: 'parent-id'})} data-testid="parent">
+                <span data-loading="show" data-testid="parent-loading-element-showing">Loading...</span>
+                <span data-loading="hide" data-testid="parent-loading-element-hiding">Loading...</span>
+                ${childTemplate({})}
+                <button data-action="live#$render">Render</button>
+            </div>
+        `);
+
+        test.expectsAjaxCall()
+            // delay so we can check loading
+            .delayResponse(20);
+
+        // All showing elements should be hidden / hiding elements should be visible
+        await waitFor(() => expect(getByTestId(test.element, 'parent-loading-element-showing')).not.toBeVisible());
+        await waitFor(() => expect(getByTestId(test.element, 'parent-loading-element-hiding')).toBeVisible());
+        await waitFor(() => expect(getByTestId(test.element, 'child-loading-element-showing')).not.toBeVisible());
+        await waitFor(() => expect(getByTestId(test.element, 'child-loading-element-hiding')).toBeVisible());
+
+        getByText(test.element, 'Render').click();
+
+        // Parent: showing elements should be visible / hiding elements should be hidden
+        expect(getByTestId(test.element, 'parent-loading-element-showing')).toBeVisible();
+        expect(getByTestId(test.element, 'parent-loading-element-hiding')).not.toBeVisible();
+        // Child: no change
+        expect(getByTestId(test.element, 'child-loading-element-showing')).not.toBeVisible();
+        expect(getByTestId(test.element, 'child-loading-element-hiding')).toBeVisible();
+
+        // wait for loading to finish
+        await (new Promise(resolve => setTimeout(resolve, 30)));
+
+        // Parent: back to original state
+        expect(getByTestId(test.element, 'parent-loading-element-showing')).not.toBeVisible();
+        expect(getByTestId(test.element, 'parent-loading-element-hiding')).toBeVisible();
+        // Child: no change
+        expect(getByTestId(test.element, 'child-loading-element-showing')).not.toBeVisible();
+        expect(getByTestId(test.element, 'child-loading-element-hiding')).toBeVisible();
+    });
 });
diff --git a/yarn.lock b/yarn.lock

Original file line number	Diff line number	Diff line change
`@@ -2370,27 +2370,27 @@ class StandardElementDriver {`
`2370`	`2370`	`class LoadingPlugin {`
`2371`	`2371`	`attachToComponent(component) {`
`2372`	`2372`	`component.on('loading.state:started', (element, request) => {`
`2373`		`- this.startLoading(element, request);`
	`2373`	`+ this.startLoading(component, element, request);`
`2374`	`2374`	`});`
`2375`	`2375`	`component.on('loading.state:finished', (element) => {`
`2376`		`- this.finishLoading(element);`
	`2376`	`+ this.finishLoading(component, element);`
`2377`	`2377`	`});`
`2378`		`- this.finishLoading(component.element);`
	`2378`	`+ this.finishLoading(component, component.element);`
`2379`	`2379`	`}`
`2380`		`- startLoading(targetElement, backendRequest) {`
`2381`		`- this.handleLoadingToggle(true, targetElement, backendRequest);`
	`2380`	`+ startLoading(component, targetElement, backendRequest) {`
	`2381`	`+ this.handleLoadingToggle(component, true, targetElement, backendRequest);`
`2382`	`2382`	`}`
`2383`		`- finishLoading(targetElement) {`
`2384`		`- this.handleLoadingToggle(false, targetElement, null);`
	`2383`	`+ finishLoading(component, targetElement) {`
	`2384`	`+ this.handleLoadingToggle(component, false, targetElement, null);`
`2385`	`2385`	`}`
`2386`		`- handleLoadingToggle(isLoading, targetElement, backendRequest) {`
	`2386`	`+ handleLoadingToggle(component, isLoading, targetElement, backendRequest) {`
`2387`	`2387`	`if (isLoading) {`
`2388`	`2388`	`this.addAttributes(targetElement, ['busy']);`
`2389`	`2389`	`}`
`2390`	`2390`	`else {`
`2391`	`2391`	`this.removeAttributes(targetElement, ['busy']);`
`2392`	`2392`	`}`
`2393`		`- this.getLoadingDirectives(targetElement).forEach(({ element, directives }) => {`
	`2393`	`+ this.getLoadingDirectives(component, targetElement).forEach(({ element, directives }) => {`
`2394`	`2394`	`if (isLoading) {`
`2395`	`2395`	`this.addAttributes(element, ['data-live-is-loading']);`
`2396`	`2396`	`}`
`@@ -2474,9 +2474,10 @@ class LoadingPlugin {`
`2474`	`2474`	`}`
`2475`	`2475`	`loadingDirective();`
`2476`	`2476`	`}`
`2477`		`- getLoadingDirectives(element) {`
	`2477`	`+ getLoadingDirectives(component, element) {`
`2478`	`2478`	`const loadingDirectives = [];`
`2479`		`- let matchingElements = element.querySelectorAll('[data-loading]');`
	`2479`	`+ let matchingElements = [...element.querySelectorAll('[data-loading]')];`
	`2480`	`+ matchingElements = matchingElements.filter((elt) => elementBelongsToThisComponent(elt, component));`
`2480`	`2481`	`if (element.hasAttribute('data-loading')) {`
`2481`	`2482`	`matchingElements = [element, ...matchingElements];`
`2482`	`2483`	`}`