Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Dialog sizing logic wait for descendants to update #5306

Merged
merged 16 commits into from
Jan 16, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 13 additions & 4 deletions components/dialog/demo/dialog-async-content.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,9 @@ import '../../list/list-item.js';
import '../../list/list-item-content.js';
import { html, LitElement } from 'lit';
import { InitialStateError, runAsync } from '../../../directives/run-async/run-async.js';
import { LoadingCompleteMixin } from '../../../mixins/loading-complete/loading-complete-mixin.js';

class DialogAsyncContent extends LitElement {
class DialogAsyncContent extends LoadingCompleteMixin(LitElement) {

static get properties() {
return {
Expand Down Expand Up @@ -32,21 +33,21 @@ class DialogAsyncContent extends LitElement {
resolve(html`
<d2l-list>
<d2l-list-item>
<img slot="illustration" src="https://s.brightspace.com/course-images/images/38e839b1-37fa-470c-8830-b189ce4ae134/tile-high-density-max-size.jpg"></img>
<img slot="illustration" src="https://s.brightspace.com/course-images/images/38e839b1-37fa-470c-8830-b189ce4ae134/tile-high-density-max-size.jpg" @load="${this.#handleImageLoad}">
<d2l-list-item-content>
<div>Introductory Earth Sciences</div>
<div slot="supporting-info">This course explores the geological process of the Earth's interior and surface. These include volcanism, earthquakes, mountains...</div>
</d2l-list-item-content>
</d2l-list-item>
<d2l-list-item>
<img slot="illustration" src="https://s.brightspace.com/course-images/images/e5fd575a-bc14-4a80-89e1-46f349a76178/tile-high-density-max-size.jpg"></img>
<img slot="illustration" src="https://s.brightspace.com/course-images/images/e5fd575a-bc14-4a80-89e1-46f349a76178/tile-high-density-max-size.jpg" @load="${this.#handleImageLoad}">
<d2l-list-item-content>
<div>Engineering Materials for Energy Systems</div>
<div slot="supporting-info">This course explores the geological processes of the Earth's interior and surface. These include volcanism, earthquakes, mountain...</div>
</d2l-list-item-content>
</d2l-list-item>
<d2l-list-item>
<img slot="illustration" src="https://s.brightspace.com/course-images/images/63b162ab-b582-4bf9-8c1d-1dad04714121/tile-high-density-max-size.jpg"></img>
<img slot="illustration" src="https://s.brightspace.com/course-images/images/63b162ab-b582-4bf9-8c1d-1dad04714121/tile-high-density-max-size.jpg" @load="${this.#handleImageLoad}">
<d2l-list-item-content>
<div>Geomorphology and GIS </div>
<div slot="supporting-info">This course explores the geological processes of the Earth's interior and surface. These include volcanism, earthquakes, mountain...</div>
Expand All @@ -58,6 +59,14 @@ class DialogAsyncContent extends LitElement {
});
}

#handleImageLoad() {
const images = this.shadowRoot.querySelectorAll('img');
for (const image of images) {
if (!image.complete) return;
KearseTrevor marked this conversation as resolved.
Show resolved Hide resolved
}
this.resolveLoadingComplete();
}

}

customElements.define('d2l-dialog-demo-async-content', DialogAsyncContent);
14 changes: 13 additions & 1 deletion components/dialog/dialog-mixin.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import '../focus-trap/focus-trap.js';
import '../../helpers/viewport-size.js';
import { allowBodyScroll, preventBodyScroll } from '../backdrop/backdrop.js';
import { clearDismissible, setDismissible } from '../../helpers/dismissible.js';
import { findComposedAncestor, isComposedAncestor } from '../../helpers/dom.js';
import { findComposedAncestor, getComposedChildren, isComposedAncestor } from '../../helpers/dom.js';
import { getComposedActiveElement, getFirstFocusableDescendant, getNextFocusable, isFocusable } from '../../helpers/focus.js';
import { classMap } from 'lit/directives/class-map.js';
import { getUniqueId } from '../../helpers/uniqueId.js';
Expand All @@ -11,6 +11,7 @@ import { ifDefined } from 'lit/directives/if-defined.js';
import { RtlMixin } from '../../mixins/rtl/rtl-mixin.js';
import { styleMap } from 'lit/directives/style-map.js';
import { tryGetIfrauBackdropService } from '../../helpers/ifrauBackdropService.js';
import { waitForElem } from '../../helpers/internal/waitForElem.js';

window.D2L = window.D2L || {};
window.D2L.DialogMixin = window.D2L.DialogMixin || {};
Expand Down Expand Up @@ -452,6 +453,11 @@ export const DialogMixin = superclass => class extends RtlMixin(superclass) {
if (reduceMotion) await new Promise(resolve => requestAnimationFrame(resolve));
else await animPromise;

const flag = window.D2L?.LP?.Web?.UI?.Flags.Flag('GAUD-7397-dialog-resize-updateComplete', true) ?? true;
if (flag) {
await this.#waitForUpdateComplete();
}
await this._updateSize();
KearseTrevor marked this conversation as resolved.
Show resolved Hide resolved
/** Dispatched when the dialog is opened */
this.dispatchEvent(new CustomEvent(
'd2l-dialog-open', { bubbles: true, composed: true }
Expand Down Expand Up @@ -580,4 +586,10 @@ export const DialogMixin = superclass => class extends RtlMixin(superclass) {
});
});
}

async #waitForUpdateComplete() {
const predicate = () => true;
const composedChildren = getComposedChildren(this, predicate);
await Promise.all(composedChildren.map(child => waitForElem(child, predicate)));
}
};
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
3 changes: 2 additions & 1 deletion helpers/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,8 @@ elemIdListRemoves(node, attrName, value);
findComposedAncestor(node, predicate);

// gets the composed children (including shadow children & distributed children)
getComposedChildren(element);
// includes a predicate which will add children nodes when predicate(node) is true
getComposedChildren(element, predicate = () => true);

// gets the composed parent (including shadow host & insertion points)
getComposedParent(node);
Expand Down
6 changes: 4 additions & 2 deletions helpers/dom.js
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ export function getBoundingAncestor(node) {
});
}

export function getComposedChildren(node) {
export function getComposedChildren(node, predicate = () => true) {

if (!node) {
return null;
Expand All @@ -104,7 +104,9 @@ export function getComposedChildren(node) {

for (let i = 0; i < nodes.length; i++) {
if (nodes[i].nodeType === 1) {
children.push(nodes[i]);
if (predicate(nodes[i])) {
children.push(nodes[i]);
}
}
}

Expand Down
25 changes: 25 additions & 0 deletions helpers/internal/waitForElem.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import { getComposedChildren } from '../dom.js';

export async function waitForElem(elem, predicate = () => true) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Worth noting that the predicate isn't currently invoked for the first/root elem passed in. If the consumer were generically iterating over an array of elements, we'd probably want to check.

If we do check the predicate for the element passed, the consumer needs to be careful which element they pass and what their predicate is.


if (!elem) return;

const update = elem.updateComplete;
if (typeof update === 'object' && Promise.resolve(update) === update) {
await update;
await new Promise(resolve => {
requestAnimationFrame(() => resolve());
});
}

if (typeof elem.getLoadingComplete === 'function') {
await elem.getLoadingComplete();
await new Promise(resolve => {
requestAnimationFrame(() => resolve());
});
}

const children = getComposedChildren(elem, predicate);
await Promise.all(children.map(e => waitForElem(e, predicate)));

}
Loading