Skip to content

Commit e4fba6a

Browse files
authored
fix(ui5-avatar-badge): handle delayed icon collection loading (#13405)
When an icon collection loader was registered but had not yet resolved, getIconDataSync() returned undefined and the sync onBeforeRendering immediately set invalid=true, permanently hiding the badge. Nothing ever cleared the flag because onBeforeRendering never re-ran. Fix: make onBeforeRendering async and fall back to await getIconData() when the sync lookup misses. The framework renders with the default invalid=false while the loader is pending, and the badge stays visible. Once the loader resolves the async continuation sets invalid correctly. Adds a Cypress regression test using a controlled deferred promise to verify the badge is never hidden while the loader is pending and the icon renders correctly after resolution. Fixes #13401
1 parent 4ac0d74 commit e4fba6a

2 files changed

Lines changed: 113 additions & 39 deletions

File tree

packages/main/cypress/specs/Avatar.cy.tsx

Lines changed: 101 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import "@ui5/webcomponents-icons/dist/accept.js";
88
import "@ui5/webcomponents-icons/dist/message-error.js";
99
import "@ui5/webcomponents-icons/dist/information.js";
1010
import "@ui5/webcomponents-icons/dist/ai.js";
11+
import { registerIconLoader } from "@ui5/webcomponents-base/dist/asset-registries/Icons.js";
1112

1213
describe("Accessibility", () => {
1314
it("checks if initials of avatar are correctly announced", () => {
@@ -31,18 +32,18 @@ describe("Accessibility", () => {
3132
const customLabel = "John Doe Avatar";
3233

3334
cy.mount(
34-
<Avatar
35-
id="interactive-info"
36-
initials={INITIALS}
37-
interactive
35+
<Avatar
36+
id="interactive-info"
37+
initials={INITIALS}
38+
interactive
3839
accessibleName={customLabel}
3940
accessibilityAttributes={{hasPopup}}
4041
></Avatar>
4142
);
4243

4344
cy.get("#interactive-info").then($avatar => {
4445
const avatar = $avatar[0] as any;
45-
46+
4647
// Check accessibilityInfo properties
4748
expect(avatar.accessibilityInfo).to.exist;
4849
expect(avatar.accessibilityInfo.role).to.equal("button");
@@ -53,15 +54,15 @@ describe("Accessibility", () => {
5354

5455
it("should return correct accessibilityInfo object when avatar is not interactive", () => {
5556
cy.mount(
56-
<Avatar
57-
id="non-interactive-info"
57+
<Avatar
58+
id="non-interactive-info"
5859
initials="JD"
5960
></Avatar>
6061
);
6162

6263
cy.get("#non-interactive-info").then($avatar => {
6364
const avatar = $avatar[0] as any;
64-
65+
6566
// Check that accessibilityInfo is undefined
6667
expect(avatar.accessibilityInfo).to.exist;
6768
expect(avatar.accessibilityInfo.role).to.equal("img");
@@ -72,18 +73,18 @@ describe("Accessibility", () => {
7273

7374
it("should use default label for accessibilityInfo description when no custom label is provided", () => {
7475
const INITIALS = "AB";
75-
76+
7677
cy.mount(
77-
<Avatar
78-
id="default-label-info"
78+
<Avatar
79+
id="default-label-info"
7980
initials={INITIALS}
8081
interactive
8182
></Avatar>
8283
);
8384

8485
cy.get("#default-label-info").then($avatar => {
8586
const avatar = $avatar[0] as any;
86-
87+
8788
// Check that accessibilityInfo uses the default label format that includes initials
8889
expect(avatar.accessibilityInfo).to.exist;
8990
expect(avatar.accessibilityInfo.description).to.equal(`Avatar ${INITIALS}`);
@@ -135,8 +136,8 @@ describe("Accessibility", () => {
135136

136137
it("mode='Decorative' renders with role='presentation' and aria-hidden", () => {
137138
cy.mount(
138-
<Avatar
139-
id="decorative-avatar"
139+
<Avatar
140+
id="decorative-avatar"
140141
initials="AB"
141142
mode="Decorative"
142143
></Avatar>
@@ -151,8 +152,8 @@ describe("Accessibility", () => {
151152

152153
it("mode='Interactive' renders with role='button' and is focusable", () => {
153154
cy.mount(
154-
<Avatar
155-
id="interactive-mode-avatar"
155+
<Avatar
156+
id="interactive-mode-avatar"
156157
initials="IJ"
157158
mode="Interactive"
158159
></Avatar>
@@ -167,10 +168,10 @@ describe("Accessibility", () => {
167168

168169
it("interactive property takes precedence over mode property", () => {
169170
cy.mount(
170-
<Avatar
171-
interactive
172-
mode="Decorative"
173-
initials="PR"
171+
<Avatar
172+
interactive
173+
mode="Decorative"
174+
initials="PR"
174175
id="precedence-avatar"
175176
></Avatar>
176177
);
@@ -186,10 +187,10 @@ describe("Accessibility", () => {
186187

187188
it("interactive=true with disabled=true renders with role='img' and is not focusable", () => {
188189
cy.mount(
189-
<Avatar
190-
interactive
191-
disabled
192-
initials="DI"
190+
<Avatar
191+
interactive
192+
disabled
193+
initials="DI"
193194
id="disabled-interactive-avatar"
194195
></Avatar>
195196
);
@@ -203,10 +204,10 @@ describe("Accessibility", () => {
203204

204205
it("mode='Interactive' with disabled=true renders with role='img' and is not focusable", () => {
205206
cy.mount(
206-
<Avatar
207+
<Avatar
207208
mode="Interactive"
208-
disabled
209-
initials="DM"
209+
disabled
210+
initials="DM"
210211
id="disabled-mode-interactive-avatar"
211212
></Avatar>
212213
);
@@ -231,7 +232,7 @@ describe("Accessibility", () => {
231232
const input = document.getElementById("mode-click-event") as HTMLInputElement;
232233
input.value = "1";
233234
}
234-
235+
235236
cy.get("#disabled-mode-click").realClick();
236237
cy.get("#mode-click-event").should("have.value", "0");
237238
});
@@ -498,7 +499,7 @@ describe("Avatar Rendering and Interaction", () => {
498499

499500
cy.get("[ui5-avatar]")
500501
.shadow()
501-
.find(".ui5-avatar-root slot:not([name])")
502+
.find(".ui5-avatar-root slot:not([name])")
502503
.should("exist");
503504

504505
cy.get("[ui5-avatar]")
@@ -607,23 +608,23 @@ describe("Avatar Rendering and Interaction", () => {
607608
.shadow()
608609
.find(".ui5-avatar-root")
609610
.realClick();
610-
611+
611612
cy.get("@clickStub")
612613
.should("have.been.calledOnce");
613614

614615
cy.get("@avatar")
615616
.shadow()
616617
.find(".ui5-avatar-root")
617618
.realPress("Enter");
618-
619+
619620
cy.get("@clickStub")
620621
.should("have.been.calledTwice");
621622

622623
cy.get("@avatar")
623624
.shadow()
624625
.find(".ui5-avatar-root")
625626
.realPress("Space");
626-
627+
627628
cy.get("@clickStub")
628629
.should("have.been.calledThrice");
629630
});
@@ -643,23 +644,23 @@ describe("Avatar Rendering and Interaction", () => {
643644
.shadow()
644645
.find(".ui5-avatar-root")
645646
.realClick();
646-
647+
647648
cy.get("@clickStub")
648649
.should("have.been.calledOnce");
649650

650651
cy.get("@avatar")
651652
.shadow()
652653
.find(".ui5-avatar-root")
653654
.realPress("Enter");
654-
655+
655656
cy.get("@clickStub")
656657
.should("have.been.calledOnce");
657658

658659
cy.get("@avatar")
659660
.shadow()
660661
.find(".ui5-avatar-root")
661662
.realPress("Space");
662-
663+
663664
cy.get("@clickStub")
664665
.should("have.been.calledOnce");
665666
});
@@ -677,7 +678,7 @@ describe("Avatar Rendering and Interaction", () => {
677678

678679
cy.get("@avatar")
679680
.realClick();
680-
681+
681682
cy.get("@clickStub")
682683
.should("have.been.calledOnce");
683684
});
@@ -879,4 +880,68 @@ describe("Avatar with Badge", () => {
879880
});
880881
});
881882
});
883+
884+
it("does not permanently hide badge when icon collection loads after initial render", () => {
885+
// Controlled deferred: loader resolves only when resolveLoader() is called.
886+
let resolveLoader!: () => void;
887+
const loaderDeferred = new Promise<void>(resolve => {
888+
resolveLoader = resolve;
889+
});
890+
891+
// Register a custom collection loader that stays pending until resolveLoader() fires.
892+
// The collection name "lazy-test-icons" is not a standard SAP collection so it passes
893+
// through getEffectiveIconCollection() unchanged, avoiding any side-effects on other tests.
894+
registerIconLoader("lazy-test-icons", () =>
895+
loaderDeferred.then(() => ({
896+
collection: "lazy-test-icons",
897+
packageName: "test",
898+
data: {
899+
"my-test-icon": {
900+
paths: ["M8 1a7 7 0 1 0 0 14A7 7 0 0 0 8 1z"],
901+
},
902+
},
903+
}))
904+
);
905+
906+
cy.mount(
907+
<Avatar initials="AB" size="M">
908+
<AvatarBadge slot="badge" icon="lazy-test-icons/my-test-icon"></AvatarBadge>
909+
</Avatar>
910+
);
911+
912+
// Wait for the first render to complete AND verify the badge is not hidden.
913+
// .ui5-avatar-badge-icon is only rendered in shadow DOM when !invalid.
914+
// Its presence proves both: (a) _render() has executed, and (b) invalid was never set to true.
915+
// With the broken sync onBeforeRendering: getIconDataSync() → undefined → invalid=true
916+
// → <Icon> is NOT rendered → this assertion FAILS.
917+
// With the async fix: the await suspends before invalid can be touched → render with
918+
// invalid=false → <Icon> IS in shadow DOM → this assertion PASSES.
919+
cy.get("[ui5-avatar-badge]")
920+
.shadow()
921+
.find(".ui5-avatar-badge-icon")
922+
.should("exist");
923+
924+
// Now resolve the loader — icon data becomes available.
925+
cy.then(() => {
926+
resolveLoader();
927+
});
928+
929+
// After resolution the badge remains visible and its inner Icon retains the name attribute.
930+
cy.get("[ui5-avatar-badge]").should("not.have.attr", "invalid");
931+
cy.get("[ui5-avatar-badge]")
932+
.shadow()
933+
.find(".ui5-avatar-badge-icon")
934+
.should("exist")
935+
.and("have.attr", "name", "lazy-test-icons/my-test-icon");
936+
937+
// Cleanup: iconCollectionPromises and registry live on the shared meta DOM element and
938+
// persist across tests in the same Cypress session. Without cleanup, a second run of this
939+
// test would find the resolved promise and icon data already in cache — getIconDataSync()
940+
// would return immediately, bypassing the deferred mechanism and masking a regression.
941+
cy.document().then(doc => {
942+
const meta = doc.querySelector("meta[name=\"ui5-shared-resources\"]") as unknown as Record<string, Record<string, Map<string, unknown>>>;
943+
meta?.["SVGIcons"]?.["promises"]?.delete("lazy-test-icons");
944+
meta?.["SVGIcons"]?.["registry"]?.delete("lazy-test-icons/my-test-icon");
945+
});
946+
});
882947
});

packages/main/src/AvatarBadge.ts

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import UI5Element from "@ui5/webcomponents-base/dist/UI5Element.js";
22
import customElement from "@ui5/webcomponents-base/dist/decorators/customElement.js";
33
import property from "@ui5/webcomponents-base/dist/decorators/property.js";
44
import jsxRenderer from "@ui5/webcomponents-base/dist/renderer/JsxRenderer.js";
5-
import { getIconDataSync } from "@ui5/webcomponents-base/dist/asset-registries/Icons.js";
5+
import { getIconData, getIconDataSync } from "@ui5/webcomponents-base/dist/asset-registries/Icons.js";
66

77
// Template
88
import AvatarBadgeTemplate from "./AvatarBadgeTemplate.js";
@@ -12,6 +12,8 @@ import AvatarBadgeCss from "./generated/themes/AvatarBadge.css.js";
1212

1313
import ValueState from "@ui5/webcomponents-base/dist/types/ValueState.js";
1414

15+
const ICON_NOT_FOUND = "ICON_NOT_FOUND";
16+
1517
/**
1618
* @class
1719
* ### Overview
@@ -83,8 +85,15 @@ class AvatarBadge extends UI5Element {
8385
@property({ type: Boolean })
8486
invalid = false;
8587

86-
onBeforeRendering() {
87-
this.invalid = !this.icon || !getIconDataSync(this.icon);
88+
async onBeforeRendering() {
89+
const icon = this.icon;
90+
if (!icon) {
91+
this.invalid = true;
92+
return;
93+
}
94+
95+
const iconData = getIconDataSync(icon) || await getIconData(icon);
96+
this.invalid = !iconData || iconData === ICON_NOT_FOUND;
8897
}
8998
}
9099

0 commit comments

Comments
 (0)