Skip to content

Commit ed57804

Browse files
weaverryanweaverryan
committed
bug #1392 [LiveComponent] Fix loadingDirectives match elements from nested components (smnandre)
This PR was squashed before being merged into the 2.x branch. Discussion ---------- [LiveComponent] Fix loadingDirectives match elements from nested components | Q | A | ------------- | --- | Bug fix? | yes | New feature? | no | Issues | Fix #1382 | License | MIT The elements with loading directives were used in all LiveController they were in. This PR fixes that and restrict loadingDirective to the scope of the LiveContoller (and filter those belonging to nested / child components) Commits ------- 7867e44 [LiveComponent] Fix loadingDirectives match elements from nested components
2 parents d933be3 + 7867e44 commit ed57804

File tree

5 files changed

+1685
-1434
lines changed

5 files changed

+1685
-1434
lines changed

‎src/LiveComponent/assets/dist/Component/plugins/LoadingPlugin.d.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,11 @@ interface ElementLoadingDirectives {
88
}
99
export default class implements PluginInterface {
1010
attachToComponent(component: Component): void;
11-
startLoading(targetElement: HTMLElement | SVGElement, backendRequest: BackendRequest): void;
12-
finishLoading(targetElement: HTMLElement | SVGElement): void;
11+
startLoading(component: Component, targetElement: HTMLElement | SVGElement, backendRequest: BackendRequest): void;
12+
finishLoading(component: Component, targetElement: HTMLElement | SVGElement): void;
1313
private handleLoadingToggle;
1414
private handleLoadingDirective;
15-
getLoadingDirectives(element: HTMLElement | SVGElement): ElementLoadingDirectives[];
15+
getLoadingDirectives(component: Component, element: HTMLElement | SVGElement): ElementLoadingDirectives[];
1616
private showElement;
1717
private hideElement;
1818
private addClass;

‎src/LiveComponent/assets/dist/live_controller.js‎

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2370,27 +2370,27 @@ class StandardElementDriver {
23702370
class LoadingPlugin {
23712371
attachToComponent(component) {
23722372
component.on('loading.state:started', (element, request) => {
2373-
this.startLoading(element, request);
2373+
this.startLoading(component, element, request);
23742374
});
23752375
component.on('loading.state:finished', (element) => {
2376-
this.finishLoading(element);
2376+
this.finishLoading(component, element);
23772377
});
2378-
this.finishLoading(component.element);
2378+
this.finishLoading(component, component.element);
23792379
}
2380-
startLoading(targetElement, backendRequest) {
2381-
this.handleLoadingToggle(true, targetElement, backendRequest);
2380+
startLoading(component, targetElement, backendRequest) {
2381+
this.handleLoadingToggle(component, true, targetElement, backendRequest);
23822382
}
2383-
finishLoading(targetElement) {
2384-
this.handleLoadingToggle(false, targetElement, null);
2383+
finishLoading(component, targetElement) {
2384+
this.handleLoadingToggle(component, false, targetElement, null);
23852385
}
2386-
handleLoadingToggle(isLoading, targetElement, backendRequest) {
2386+
handleLoadingToggle(component, isLoading, targetElement, backendRequest) {
23872387
if (isLoading) {
23882388
this.addAttributes(targetElement, ['busy']);
23892389
}
23902390
else {
23912391
this.removeAttributes(targetElement, ['busy']);
23922392
}
2393-
this.getLoadingDirectives(targetElement).forEach(({ element, directives }) => {
2393+
this.getLoadingDirectives(component, targetElement).forEach(({ element, directives }) => {
23942394
if (isLoading) {
23952395
this.addAttributes(element, ['data-live-is-loading']);
23962396
}
@@ -2474,9 +2474,10 @@ class LoadingPlugin {
24742474
}
24752475
loadingDirective();
24762476
}
2477-
getLoadingDirectives(element) {
2477+
getLoadingDirectives(component, element) {
24782478
const loadingDirectives = [];
2479-
let matchingElements = element.querySelectorAll('[data-loading]');
2479+
let matchingElements = [...element.querySelectorAll('[data-loading]')];
2480+
matchingElements = matchingElements.filter((elt) => elementBelongsToThisComponent(elt, component));
24802481
if (element.hasAttribute('data-loading')) {
24812482
matchingElements = [element, ...matchingElements];
24822483
}

‎src/LiveComponent/assets/src/Component/plugins/LoadingPlugin.ts‎

Lines changed: 15 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import {
33
DirectiveModifier,
44
parseDirectives
55
} from '../../Directive/directives_parser';
6+
import { elementBelongsToThisComponent } from '../../dom_utils';
67
import { combineSpacedArray } from '../../string_utils';
78
import BackendRequest from '../../Backend/BackendRequest';
89
import Component from '../../Component';
@@ -16,32 +17,32 @@ interface ElementLoadingDirectives {
1617
export default class implements PluginInterface {
1718
attachToComponent(component: Component): void {
1819
component.on('loading.state:started', (element: HTMLElement, request: BackendRequest) => {
19-
this.startLoading(element, request);
20+
this.startLoading(component, element, request);
2021
});
2122
component.on('loading.state:finished', (element: HTMLElement) => {
22-
this.finishLoading(element);
23+
this.finishLoading(component, element);
2324
});
2425
// hide "loading" elements to begin with
2526
// This is done with CSS, but only for the most basic cases
26-
this.finishLoading(component.element);
27+
this.finishLoading(component, component.element);
2728
}
2829

29-
startLoading(targetElement: HTMLElement|SVGElement, backendRequest: BackendRequest): void {
30-
this.handleLoadingToggle(true, targetElement, backendRequest);
30+
startLoading(component: Component, targetElement: HTMLElement|SVGElement, backendRequest: BackendRequest): void {
31+
this.handleLoadingToggle(component, true, targetElement, backendRequest);
3132
}
3233

33-
finishLoading(targetElement: HTMLElement|SVGElement): void {
34-
this.handleLoadingToggle(false, targetElement, null);
34+
finishLoading(component: Component, targetElement: HTMLElement|SVGElement): void {
35+
this.handleLoadingToggle(component,false, targetElement, null);
3536
}
3637

37-
private handleLoadingToggle(isLoading: boolean, targetElement: HTMLElement|SVGElement, backendRequest: BackendRequest|null) {
38+
private handleLoadingToggle(component: Component, isLoading: boolean, targetElement: HTMLElement|SVGElement, backendRequest: BackendRequest|null) {
3839
if (isLoading) {
3940
this.addAttributes(targetElement, ['busy']);
4041
} else {
4142
this.removeAttributes(targetElement, ['busy']);
4243
}
4344

44-
this.getLoadingDirectives(targetElement).forEach(({ element, directives }) => {
45+
this.getLoadingDirectives(component, targetElement).forEach(({ element, directives }) => {
4546
// so we can track, at any point, if an element is in a "loading" state
4647
if (isLoading) {
4748
this.addAttributes(element, ['data-live-is-loading']);
@@ -148,9 +149,12 @@ export default class implements PluginInterface {
148149
loadingDirective();
149150
}
150151

151-
getLoadingDirectives(element: HTMLElement|SVGElement) {
152+
getLoadingDirectives(component: Component, element: HTMLElement|SVGElement) {
152153
const loadingDirectives: ElementLoadingDirectives[] = [];
153-
let matchingElements = element.querySelectorAll('[data-loading]');
154+
let matchingElements = [...element.querySelectorAll('[data-loading]')];
155+
156+
// ignore elements which are inside a nested "live" component
157+
matchingElements = matchingElements.filter((elt) => elementBelongsToThisComponent(elt, component));
154158

155159
// querySelectorAll doesn't include the element itself
156160
if (element.hasAttribute('data-loading')) {

‎src/LiveComponent/assets/test/controller/loading.test.ts‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -223,4 +223,51 @@ describe('LiveController data-loading Tests', () => {
223223
await (new Promise(resolve => setTimeout(resolve, 30)));
224224
expect(getByTestId(test.element, 'loading-element')).not.toBeVisible();
225225
});
226+
227+
it('does not trigger loading inside component children', async () => {
228+
const childTemplate = (data: any) => `
229+
<div ${initComponent(data, {id: 'child-id'})} data-testid="child">
230+
<span data-loading="show" data-testid="child-loading-element-showing">Loading...</span>
231+
<span data-loading="hide" data-testid="child-loading-element-hiding">Loading...</span>
232+
</div>
233+
`;
234+
235+
const test = await createTest({} , (data: any) => `
236+
<div ${initComponent(data, {id: 'parent-id'})} data-testid="parent">
237+
<span data-loading="show" data-testid="parent-loading-element-showing">Loading...</span>
238+
<span data-loading="hide" data-testid="parent-loading-element-hiding">Loading...</span>
239+
${childTemplate({})}
240+
<button data-action="live#$render">Render</button>
241+
</div>
242+
`);
243+
244+
test.expectsAjaxCall()
245+
// delay so we can check loading
246+
.delayResponse(20);
247+
248+
// All showing elements should be hidden / hiding elements should be visible
249+
await waitFor(() => expect(getByTestId(test.element, 'parent-loading-element-showing')).not.toBeVisible());
250+
await waitFor(() => expect(getByTestId(test.element, 'parent-loading-element-hiding')).toBeVisible());
251+
await waitFor(() => expect(getByTestId(test.element, 'child-loading-element-showing')).not.toBeVisible());
252+
await waitFor(() => expect(getByTestId(test.element, 'child-loading-element-hiding')).toBeVisible());
253+
254+
getByText(test.element, 'Render').click();
255+
256+
// Parent: showing elements should be visible / hiding elements should be hidden
257+
expect(getByTestId(test.element, 'parent-loading-element-showing')).toBeVisible();
258+
expect(getByTestId(test.element, 'parent-loading-element-hiding')).not.toBeVisible();
259+
// Child: no change
260+
expect(getByTestId(test.element, 'child-loading-element-showing')).not.toBeVisible();
261+
expect(getByTestId(test.element, 'child-loading-element-hiding')).toBeVisible();
262+
263+
// wait for loading to finish
264+
await (new Promise(resolve => setTimeout(resolve, 30)));
265+
266+
// Parent: back to original state
267+
expect(getByTestId(test.element, 'parent-loading-element-showing')).not.toBeVisible();
268+
expect(getByTestId(test.element, 'parent-loading-element-hiding')).toBeVisible();
269+
// Child: no change
270+
expect(getByTestId(test.element, 'child-loading-element-showing')).not.toBeVisible();
271+
expect(getByTestId(test.element, 'child-loading-element-hiding')).toBeVisible();
272+
});
226273
});

0 commit comments

Comments
 (0)