-
Notifications
You must be signed in to change notification settings - Fork 279
feat(ui5-dynamic-page): add accessibilityAttributes property #13484
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
base: main
Are you sure you want to change the base?
Changes from all commits
039f63b
16e6ace
1db8725
3e02a33
4ce3cc8
ac830b3
7336638
0f81e09
003146c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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"; | ||
|
|
@@ -32,6 +33,14 @@ import { | |
|
|
||
| import type { Slot, DefaultSlot } from "@ui5/webcomponents-base/dist/UI5Element.js"; | ||
|
|
||
| type DynamicPageAccessibilityAttributes = { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consider narrowing
As is, a developer can set e.g. |
||
| root?: { role?: AriaLandmarkRole; name?: string }; | ||
| header?: { role?: AriaLandmarkRole; name?: string }; | ||
| headerContent?: { name?: string }; | ||
| content?: { role?: AriaLandmarkRole; name?: string }; | ||
| footer?: { role?: AriaLandmarkRole; name?: string }; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggest exposing a public |
||
| }; | ||
|
|
||
| const SCROLL_DEBOUNCE_RATE = 5; // ms | ||
| const SCROLL_THRESHOLD = 10; // px | ||
| /** | ||
|
|
@@ -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 {} | ||
| */ | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. JSDoc style differs from Also double-check |
||
| @property({ type: Object }) | ||
| accessibilityAttributes: DynamicPageAccessibilityAttributes = {}; | ||
|
|
||
| @i18n("@ui5/webcomponents-fiori") | ||
| static i18nBundle: I18nBundle; | ||
|
|
||
|
|
@@ -213,6 +239,7 @@ class DynamicPage extends UI5Element { | |
| } | ||
| if (this.dynamicPageHeader) { | ||
| this.dynamicPageHeader._snapped = this._headerSnapped; | ||
| this.dynamicPageHeader._accessibleName = this.accessibilityAttributes.headerContent?.name; | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -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(); | ||
| } | ||
|
|
@@ -480,3 +515,5 @@ class DynamicPage extends UI5Element { | |
| DynamicPage.define(); | ||
|
|
||
| export default DynamicPage; | ||
|
|
||
| export type { DynamicPageAccessibilityAttributes }; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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} | ||
|
|
@@ -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} | ||
| > | ||
|
|
@@ -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> | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Alternative: always render |
||
| ) : ( | ||
| <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 ( | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -183,6 +183,11 @@ | |
| const cancelEdit = document.querySelector("#cancel-edit"); | ||
| const saveEdit = document.querySelector("#save-edit"); | ||
|
|
||
| dynamicPage.accessibilityAttributes = { | ||
| header: { role: "none" }, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This unconditionally applies |
||
| content: { role: "main" }, | ||
| }; | ||
|
|
||
| editButton.addEventListener("click", () => { | ||
| dynamicPage.setAttribute("show-footer", true); | ||
| }); | ||
|
|
||
There was a problem hiding this comment.
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 setsrole: "none"). There is no test verifying that the default<header>element is still rendered when onlyheader.nameis set. Please add:…with only
header.nameset, to lock in the no-role-override path.