Skip to content
Open
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
75 changes: 75 additions & 0 deletions packages/fiori/cypress/specs/DynamicPage.cy.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1121,4 +1121,79 @@ describe("ARIA attributes", () => {
.find(".ui5-dynamic-page-header-root")
.should("have.attr", "aria-label", "Header Expanded");
});

it("supports customizing header role and label via accessibilityAttributes", () => {
cy.mount(
<DynamicPage style={{ height: "600px" }}>
<DynamicPageTitle slot="titleArea">
<div slot="heading">Page Title</div>
</DynamicPageTitle>
<DynamicPageHeader slot="headerArea">
<div>Header Content</div>
</DynamicPageHeader>
<div style={{ height: "1000px" }}>Content</div>
</DynamicPage>
);

cy.get("[ui5-dynamic-page]").invoke("prop", "accessibilityAttributes", {
header: { role: "none", name: "Custom Header" },
});

cy.get("[ui5-dynamic-page]")
.shadow()
.find(".ui5-dynamic-page-title-header-wrapper")
.should("have.attr", "role", "none")
.should("have.attr", "aria-label", "Custom Header");
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This test exercises the <div role="none"> branch (line 1135 sets role: "none"). There is no test verifying that the default <header> element is still rendered when only header.name is set. Please add:

cy.get("[ui5-dynamic-page]").shadow().find("header.ui5-dynamic-page-title-header-wrapper").should("exist");

…with only header.name set, to lock in the no-role-override path.

});

it("supports customizing headerContent label via accessibilityAttributes", () => {
cy.mount(
<DynamicPage style={{ height: "600px" }}>
<DynamicPageTitle slot="titleArea">
<div slot="heading">Page Title</div>
</DynamicPageTitle>
<DynamicPageHeader slot="headerArea">
<div>Header Content</div>
</DynamicPageHeader>
<div style={{ height: "1000px" }}>Content</div>
</DynamicPage>
);

cy.get("[ui5-dynamic-page]").invoke("prop", "accessibilityAttributes", {
headerContent: { name: "Custom Region Label" },
});

cy.get("[ui5-dynamic-page-header]")
.shadow()
.find(".ui5-dynamic-page-header-root")
.should("have.attr", "aria-label", "Custom Region Label");
});

it("supports customizing content and footer roles via accessibilityAttributes", () => {
cy.mount(
<DynamicPage style={{ height: "600px" }}>
<DynamicPageTitle slot="titleArea">
<div slot="heading">Page Title</div>
</DynamicPageTitle>
<div style={{ height: "1000px" }}>Content</div>
</DynamicPage>
);

cy.get("[ui5-dynamic-page]").invoke("prop", "accessibilityAttributes", {
content: { role: "main", name: "Page Content" },
footer: { role: "contentinfo", name: "Page Footer" },
});

cy.get("[ui5-dynamic-page]")
.shadow()
.find(".ui5-dynamic-page-content")
.should("have.attr", "role", "main")
.should("have.attr", "aria-label", "Page Content");

cy.get("[ui5-dynamic-page]")
.shadow()
.find(".ui5-dynamic-page-footer")
.should("have.attr", "role", "contentinfo")
.should("have.attr", "aria-label", "Page Footer");
});
});
39 changes: 38 additions & 1 deletion packages/fiori/src/DynamicPage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { renderFinished } from "@ui5/webcomponents-base/dist/Render.js";
import announce from "@ui5/webcomponents-base/dist/util/InvisibleMessage.js";
import InvisibleMessageMode from "@ui5/webcomponents-base/dist/types/InvisibleMessageMode.js";
import type I18nBundle from "@ui5/webcomponents-base/dist/i18nBundle.js";
import type { AriaLandmarkRole } from "@ui5/webcomponents-base";
import { isPhone } from "@ui5/webcomponents-base/dist/Device.js";

import debounce from "@ui5/webcomponents-base/dist/util/debounce.js";
Expand All @@ -32,6 +33,14 @@ import {

import type { Slot, DefaultSlot } from "@ui5/webcomponents-base/dist/UI5Element.js";

type DynamicPageAccessibilityAttributes = {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Consider narrowing role per section instead of allowing the full AriaLandmarkRole union for every region. FlexibleColumnLayout.ts:111 already uses Extract<AriaLandmarkRole, ...> for the same reason. Suggested narrowing:

  • header: "none" | "banner" | "region"
  • content: "none" | "main" | "region" | "form"
  • footer: "none" | "contentinfo" | "region"
  • root: "none" | "main" | "region"

As is, a developer can set e.g. footer.role = "banner" or content.role = "search", which are nonsensical and will pass typecheck.

root?: { role?: AriaLandmarkRole; name?: string };
header?: { role?: AriaLandmarkRole; name?: string };
headerContent?: { name?: string };
content?: { role?: AriaLandmarkRole; name?: string };
footer?: { role?: AriaLandmarkRole; name?: string };
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

headerContent is asymmetric (only name, no role) because DynamicPageHeader is a separate component with its own role="region". Two concerns:

  1. Consumers reading the API will reasonably expect headerContent to mirror the header shape.
  2. The override flows through a private _accessibleName property set in onBeforeRendering (line 242) — a side-channel that is easy to miss and re-renders only when DynamicPage itself re-renders.

Suggest exposing a public accessibleName directly on DynamicPageHeader instead. Cleaner, consistent with how every other component handles accessibleName, and DynamicPage doesn't need to reach into the child.

};

const SCROLL_DEBOUNCE_RATE = 5; // ms
const SCROLL_THRESHOLD = 10; // px
/**
Expand Down Expand Up @@ -184,6 +193,23 @@ class DynamicPage extends UI5Element {
@slot({ type: HTMLElement })
footerArea!: Slot<HTMLElement>;

/**
* Defines the accessibility attributes for DynamicPage sections.
*
* Accepted fields per section — `root`, `header`, `content`, `footer`:
* - `role`: Overrides the ARIA landmark role. Accepts the following string values: `"none"`, `"banner"`, `"main"`, `"region"`, `"navigation"`, `"search"`, `"complementary"`, `"form"`, `"contentinfo"`.
* - `name`: Sets `aria-label` on the section. Accepts any string.
*
* Accepted fields for `headerContent`:
* - `name`: Sets `aria-label` on the DynamicPageHeader region (overrides the default "Header Expanded"/"Header Snapped" text). Accepts any string.
*
* @public
* @since 2.23.0
* @default {}
*/
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

JSDoc style differs from FlexibleColumnLayout.accessibilityAttributes (FCL splits role and name into separate sections with the full list of accepted values per attribute). Please align with the FCL doc style for consistency across the design system.

Also double-check @since 2.23.0 against the next planned release tag — confirm 2.23.0 is the upcoming version and not 2.24.0.

@property({ type: Object })
accessibilityAttributes: DynamicPageAccessibilityAttributes = {};

@i18n("@ui5/webcomponents-fiori")
static i18nBundle: I18nBundle;

Expand Down Expand Up @@ -213,6 +239,7 @@ class DynamicPage extends UI5Element {
}
if (this.dynamicPageHeader) {
this.dynamicPageHeader._snapped = this._headerSnapped;
this.dynamicPageHeader._accessibleName = this.accessibilityAttributes.headerContent?.name;
}
}

Expand Down Expand Up @@ -281,9 +308,17 @@ class DynamicPage extends UI5Element {
}

get headerAriaLabel() {
return this.hasHeading ? this._headerLabel : undefined;
return this.accessibilityAttributes.header?.name || (this.hasHeading ? this._headerLabel : undefined);
}

get _headerRole() { return this.accessibilityAttributes.header?.role; }
get _rootRole() { return this.accessibilityAttributes.root?.role; }
get _rootAriaLabel() { return this.accessibilityAttributes.root?.name; }
get _contentRole() { return this.accessibilityAttributes.content?.role; }
get _contentAriaLabel() { return this.accessibilityAttributes.content?.name; }
get _footerRole() { return this.accessibilityAttributes.footer?.role; }
get _footerAriaLabel() { return this.accessibilityAttributes.footer?.name; }

get _hidePinButton() {
return this.hidePinButton || isPhone();
}
Expand Down Expand Up @@ -480,3 +515,5 @@ class DynamicPage extends UI5Element {
DynamicPage.define();

export default DynamicPage;

export type { DynamicPageAccessibilityAttributes };
9 changes: 9 additions & 0 deletions packages/fiori/src/DynamicPageHeader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,12 @@ class DynamicPageHeader extends UI5Element {
@property({ type: Boolean })
_snapped = false;

/**
* @private
*/
@property()
_accessibleName?: string;

@i18n("@ui5/webcomponents-fiori")
static i18nBundle: I18nBundle;

Expand All @@ -83,6 +89,9 @@ class DynamicPageHeader extends UI5Element {
* @internal
*/
get _headerRegionAriaLabel(): string {
if (this._accessibleName) {
return this._accessibleName;
}
const defaultText = this._snapped
? DYNAMIC_PAGE_ARIA_LABEL_SNAPPED_HEADER
: DYNAMIC_PAGE_ARIA_LABEL_EXPANDED_HEADER;
Expand Down
57 changes: 39 additions & 18 deletions packages/fiori/src/DynamicPageTemplate.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,26 +3,11 @@ import DynamicPageHeaderActions from "./DynamicPageHeaderActions.js";

export default function DynamicPageTemplate(this: DynamicPage) {
return (
<div class="ui5-dynamic-page-root">
<div class="ui5-dynamic-page-root" role={this._rootRole} aria-label={this._rootAriaLabel}>
<div class="ui5-dynamic-page-scroll-container"
onScroll={this.snapOnScroll}
>
<header
class="ui5-dynamic-page-title-header-wrapper"
id={`${this._id}-header`}
aria-label={this.headerAriaLabel}
onui5-toggle-title={this.onToggleTitle}
>
<slot name="titleArea"></slot>
{this.headerInTitle &&
<slot tabIndex={this.headerTabIndex}
aria-hidden={this.headerAriaHidden}
name="headerArea"
></slot>
}

{this.actionsInTitle && headerActions.call(this)}
</header>
{titleHeaderWrapper.call(this)}

{this.headerInContent &&
<slot tabIndex={this.headerTabIndex}
Expand All @@ -36,6 +21,8 @@ export default function DynamicPageTemplate(this: DynamicPage) {
<div
part="content"
class="ui5-dynamic-page-content"
role={this._contentRole}
aria-label={this._contentAriaLabel}
onFocusIn={this.onContentFocusIn}
onFocusOut={this.onContentFocusOut}
>
Expand All @@ -48,13 +35,47 @@ export default function DynamicPageTemplate(this: DynamicPage) {
</div>
</div>

<div class="ui5-dynamic-page-footer" part="footer">
<div class="ui5-dynamic-page-footer" part="footer" role={this._footerRole} aria-label={this._footerAriaLabel}>
<slot name="footerArea"></slot>
</div>
</div>
);
}

function titleHeaderWrapper(this: DynamicPage) {
const commonProps = {
"class": "ui5-dynamic-page-title-header-wrapper",
id: `${this._id}-header`,
"aria-label": this.headerAriaLabel,
"onui5-toggle-title": this.onToggleTitle,
};

return this._headerRole ? (
<div {...commonProps} role={this._headerRole}>
{titleHeaderContent.call(this)}
</div>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Alternative: always render <div role={this._headerRole || "banner"}>, removing the helper and the conditional. This keeps the markup consistent and removes a code path. Up to you - current solution works, just adds branching.

) : (
<header {...commonProps}>
{titleHeaderContent.call(this)}
</header>
);
}

function titleHeaderContent(this: DynamicPage) {
return (
<>
<slot name="titleArea"></slot>
{this.headerInTitle &&
<slot tabIndex={this.headerTabIndex}
aria-hidden={this.headerAriaHidden}
name="headerArea"
></slot>
}
{this.actionsInTitle && headerActions.call(this)}
</>
);
}

function headerActions(this: DynamicPage) {
if (!this.hasSnappedTitleOnMobile && this.hasHeading) {
return (
Expand Down
5 changes: 5 additions & 0 deletions packages/fiori/test/pages/DynamicPage.html
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,11 @@
const cancelEdit = document.querySelector("#cancel-edit");
const saveEdit = document.querySelector("#save-edit");

dynamicPage.accessibilityAttributes = {
header: { role: "none" },
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This unconditionally applies header: { role: "none" }, content: { role: "main" } on the demo page, which masks the default landmark structure for anyone visually verifying the page. Either revert this change, or make it opt-in (e.g. via a checkbox/button) so the default rendering remains testable from the demo page.

content: { role: "main" },
};

editButton.addEventListener("click", () => {
dynamicPage.setAttribute("show-footer", true);
});
Expand Down
Loading