Skip to content
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
114 changes: 114 additions & 0 deletions packages/fiori/cypress/specs/IllustratedMessage.cy.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -505,4 +505,118 @@ describe("Utility SVG accessibility", () => {
expect($svg).not.to.have.attr("focusable");
});
});
});

describe("Media oscillation prevention", () => {
// Regression for the endless A -> B -> A -> ... -> sequence of media changes:
// when the illustrated message sits in an auto-height wrapper inside a scrollable
// container whose width is a few px above the breakpoint between A and B
// and change in media brings change in height which toggles the scrollbar
// which brings the width back and forth across the breakpoint, potentially causing media oscillation.
it("settles on a stable media after resizing from above to just above the scene/dialog breakpoint", () => {
cy.mount(
<div
id="osc-box"
style={{ width: "740px", height: "360px", overflow: "auto" }}
>
<div>
<IllustratedMessage name="NoData" design="Auto" titleText="No data" subtitleText="Nothing to show" />
</div>
</div>
);

cy.get("[ui5-illustrated-message]").should("have.attr", "media", IllustratedMessage.MEDIA.SCENE);

// Shrink to the oscillation-trigger width.
cy.get("#osc-box").invoke("css", "width", "696px");

cy.get("[ui5-illustrated-message]").then(($im) => {
const im = $im[0];
const transitions: string[] = [];
const observer = new MutationObserver(() => {
transitions.push(im.getAttribute("media") || "");
});
observer.observe(im, { attributes: true, attributeFilter: ["media"] });
cy.wrap(transitions).as("transitions");
cy.wrap(observer).as("observer");
});

cy.wait(2000);

cy.get("[ui5-illustrated-message]")
.invoke("attr", "media")
.then((settledMedia) => {
cy.get<string[]>("@transitions").then((transitions) => {
const countAfterSettle = transitions.length;

cy.wait(1000);

cy.get("[ui5-illustrated-message]")
.should("have.attr", "media", settledMedia as string);

cy.get<string[]>("@transitions").then((t) => {
expect(t.length, "no further media transitions after settling").to.equal(countAfterSettle);
});
});
});

cy.get<MutationObserver>("@observer").then((observer) => {
observer.disconnect();
});
});

it("settles on a stable media instead of oscillating", () => {
cy.mount(
// #box: width a few px above the scene breakpoint; height between the dialog and
// scene content heights, so a non-overlay scrollbar moves the available width across
// the breakpoint. Inner wrapper is auto-height so overflow surfaces on #box.
<div
id="osc-box"
style={{ width: "696px", height: "360px", overflow: "auto" }}
>
<div>
<IllustratedMessage name="NoData" design="Auto" titleText="No data" subtitleText="Nothing to show" />
</div>
</div>
);

// Record every media attribute change via a MutationObserver on the host.
cy.get("[ui5-illustrated-message]").then(($im) => {
const im = $im[0];
const transitions: string[] = [];
const observer = new MutationObserver(() => {
transitions.push(im.getAttribute("media") || "");
});
observer.observe(im, { attributes: true, attributeFilter: ["media"] });
cy.wrap(transitions).as("transitions");
cy.wrap(observer).as("observer");
});

// Give the resize/render feedback loop ample time to run. If the bug is present the
// transition count keeps growing without bound during this window.
cy.wait(2000);

// Snapshot the current media and the transition count, wait again, and assert nothing
// moved: media is identical and no further transitions were recorded.
cy.get("[ui5-illustrated-message]")
.invoke("attr", "media")
.then((settledMedia) => {
cy.get<string[]>("@transitions").then((transitions) => {
const countAfterSettle = transitions.length;

cy.wait(1000);

cy.get("[ui5-illustrated-message]")
.should("have.attr", "media", settledMedia as string);

cy.get<string[]>("@transitions").then((t) => {
expect(t.length, "no further media transitions after settling").to.equal(countAfterSettle);
});
});
});

cy.get<MutationObserver>("@observer").then((observer) => {
observer.disconnect();
});
});
});
79 changes: 71 additions & 8 deletions packages/fiori/src/IllustratedMessage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,21 @@ const getEffectiveIllustrationName = (name: string): string => {
return `fiori/${name}`;
};

const MEDIA = {
BASE: "base",
DOT: "dot",
SPOT: "spot",
DIALOG: "dialog",
SCENE: "scene",
} as const;

type Media = typeof MEDIA[keyof typeof MEDIA];

type DimensionsForMedia = {
beforeRendering: { width: number; height: number; },
afterRendering?: { width: number; height: number; },
};

/**
* @class
*
Expand Down Expand Up @@ -277,6 +292,9 @@ class IllustratedMessage extends UI5Element {
@i18n("@ui5/webcomponents-fiori")
static i18nBundle: I18nBundle;
_contentHeightForMedia: Record<string, number>;
// tracks changes to the `media` property; cleared when the rendered media brings no further resizing;
// required to prevent a circular chain of `media` changes (A -> B -> A -> ...) where a media change triggers a resize that reverts it to a previous media of same chained sequence.
_ongoingMediaChange: Array<{ media: Media; dimensions: DimensionsForMedia }>;
_handleResize: ResizeObserverCallback;
_handleThemeLoaded: () => void;

Expand All @@ -291,6 +309,7 @@ class IllustratedMessage extends UI5Element {
};
// this will store the height of the inner content of the IllustratedMessage (illustration + title + subtitle + actions) for a given media (e.g. "Spot")
this._contentHeightForMedia = {};
this._ongoingMediaChange = [];
}

static get BREAKPOINTS() {
Expand All @@ -303,13 +322,7 @@ class IllustratedMessage extends UI5Element {
}

static get MEDIA() {
return {
BASE: "base",
DOT: "dot",
SPOT: "spot",
DIALOG: "dialog",
SCENE: "scene",
};
return MEDIA;
}

async onBeforeRendering() {
Expand Down Expand Up @@ -409,7 +422,8 @@ class IllustratedMessage extends UI5Element {

_applyMedia() {
const width = this.offsetWidth;
let media = "",
const height = this.offsetHeight;
let media: Media = MEDIA.SCENE,
mediaIndex = -1;

if (width <= IllustratedMessage.BREAKPOINTS.BASE) {
Expand All @@ -431,9 +445,55 @@ class IllustratedMessage extends UI5Element {
media = Object.values(IllustratedMessage.MEDIA)[mediaIndex];
}

if (this.media && this._wouldRevertLastMediaChange(media, width, height)) {
// circular chain of media changes detected, so settle on the currently applied media to escape oscillation
return;
}

if (media === this.media) {
return; // no media change
}

this._ongoingMediaChange.push({ media, dimensions: { beforeRendering: { width, height } } }); // register media change step
Comment thread
kineticjs marked this conversation as resolved.
this._ongoingMediaChange = this._ongoingMediaChange.slice(-2); // we need to keep only the last two steps to detect oscillation
this.media = media;
}

_wouldRevertLastMediaChange(media: Media, newWidth: number, newHeight: number): boolean {
const steps = this._ongoingMediaChange;
const currentAppliedMedia = steps.length >= 1 ? steps[steps.length - 1] : null;
const previousAppliedMedia = steps.length >= 2 ? steps[steps.length - 2] : null;
const newDimensions = { width: newWidth, height: newHeight };

if (previousAppliedMedia?.media !== media
|| !currentAppliedMedia?.dimensions.afterRendering
|| !previousAppliedMedia?.dimensions.afterRendering) {
return false;
}

// circular chain: previousApplied -> currentApplied -> previousApplied
return this._dimensionsMatch(previousAppliedMedia.dimensions.beforeRendering, newDimensions)
&& this._dimensionsMatch(previousAppliedMedia.dimensions.afterRendering, currentAppliedMedia.dimensions.beforeRendering)
&& this._dimensionsMatch(currentAppliedMedia.dimensions.afterRendering, newDimensions);
}

_dimensionsMatch(dimensions1: { width: number; height: number }, dimensions2: { width: number; height: number }): boolean {
return dimensions1.width === dimensions2.width
&& dimensions1.height === dimensions2.height;
}

_trackMediaAfterRendering() {
const steps = this._ongoingMediaChange;
const lastStep = steps.length > 0 ? steps[steps.length - 1] : null;
if (lastStep && this.media as Media === lastStep.media) {
lastStep.dimensions.afterRendering = { width: this.offsetWidth, height: this.offsetHeight };

if (this._dimensionsMatch(lastStep.dimensions.beforeRendering, lastStep.dimensions.afterRendering)) {
this._ongoingMediaChange = []; // media settled (no further resize triggered)
}
}
}

_mediaExceedsContainerHeight(media: string): boolean {
return !!this._contentHeightForMedia[media] && this.clientHeight < this._contentHeightForMedia[media];
}
Expand Down Expand Up @@ -467,6 +527,9 @@ class IllustratedMessage extends UI5Element {
if (this.design !== IllustrationMessageDesign.Auto) {
return;
}

this._trackMediaAfterRendering();

const heightMeasurementNeeded = this.media && !(this.media in this._contentHeightForMedia);
const mightOverflow = this.scrollHeight > this.clientHeight;
if (heightMeasurementNeeded || mightOverflow) {
Expand Down
66 changes: 66 additions & 0 deletions packages/fiori/test/pages/IllustratedMessage.html
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,46 @@ <h2>Vertical responsiveness</h2>
</ui5-illustrated-message>
</div>

<h2>Width between the scene/dialog breakpoint (must not cause endless scrollbar toggle)</h2>
<!-- Reproduces conditions for the endless scene<->dialog loop: an auto-height wrapper inside a scrollable
container whose width is a few px above the scene breakpoint (681px) and whose height falls
between the dialog and scene content heights. Choosing "scene" grows the content, a classic
scrollbar appears and steals ~15px of width, pushing it below the breakpoint to "dialog";
"dialog" removes the overflow, the scrollbar disappears, width grows back and "scene" is
chosen again. The on-page log must settle after a bounded number of transitions. -->

<div class="osc-controls">
<label for="oscSlider">Box width: <span id="oscSliderValue">696</span>px</label>
<div class="osc-slider-row">
<span class="osc-slider-min">400</span>
<div class="osc-slider-track">
<input id="oscSlider" type="range" min="400" max="900" value="696" step="1">
<!-- tick marks for the 3 critical presets -->
<div class="osc-ticks">
<span class="osc-tick" style="left:calc((640 - 400) / 500 * 100%)" title="640px"></span>
<span class="osc-tick osc-tick--warn" style="left:calc((696 - 400) / 500 * 100%)" title="696px"></span>
<span class="osc-tick" style="left:calc((740 - 400) / 500 * 100%)" title="740px"></span>
</div>
</div>
<span class="osc-slider-max">900</span>
</div>
</div>

<div id="oscBox" class="osc-box border">
<!-- auto-height wrapper (table cell, card body, ...): the message grows with its content,
so its overflow surfaces as a scrollbar on #oscBox -->
<div>
<ui5-illustrated-message
id="illustratedMsg6"
name="NoData"
design="Auto"
title-text="No data"
subtitle-text="Nothing to show"
></ui5-illustrated-message>
</div>
</div>
<div id="oscLog" class="osc-log"></div>

<script type="module">
import { setTheme } from "@ui5/webcomponents-base/dist/config/Theme.js";

Expand Down Expand Up @@ -262,6 +302,32 @@ <h2>Vertical responsiveness</h2>
containerThemeSelect.addEventListener("ui5-change", (event) => {
setTheme(event.detail.selectedOption.textContent);
});

// Oscillation box width controls
const oscBox = document.getElementById("oscBox");
const oscSlider = document.getElementById("oscSlider");
const oscSliderValue = document.getElementById("oscSliderValue");

const setOscWidth = (px) => {
oscBox.style.width = `${px}px`;
oscSlider.value = px;
oscSliderValue.textContent = px;
};

oscSlider.addEventListener("input", () => {
setOscWidth(Number(oscSlider.value));
});

// Detect media oscillation: log every media transition so a regression (endless flipping) is
// visible on the page. With the fix, the log settles after a bounded number of lines.
const oscMessage = document.getElementById("illustratedMsg6");
const oscLog = document.getElementById("oscLog");
const oscOut = (line) => {
oscLog.textContent += line + "\n";
};
new MutationObserver(() => {
oscOut(`media -> ${oscMessage.getAttribute("media")} (width ${oscMessage.offsetWidth}px)`);
}).observe(oscMessage, { attributes: true, attributeFilter: ["media"] });
</script>
</body>

Expand Down
Loading
Loading