From 49c7cb2ef7219fd219a1e0061d46155ce1e82ba0 Mon Sep 17 00:00:00 2001 From: felixw Date: Fri, 20 Oct 2023 16:11:01 +0200 Subject: [PATCH 1/9] fix: aria role behavior --- .../src/components/notification/notification.tsx | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/packages/components/src/components/notification/notification.tsx b/packages/components/src/components/notification/notification.tsx index 15ed7af6b4..9084499aa3 100644 --- a/packages/components/src/components/notification/notification.tsx +++ b/packages/components/src/components/notification/notification.tsx @@ -59,8 +59,10 @@ export class Notification { @Prop() dismissible?: boolean = false; /** (optional) Time in milliseconds until it closes by itself */ @Prop() delay?: number; - /** (optional) `aria-live` of element */ + /** @deprecated - ariaRole should replace innerAriaLive */ @Prop() innerAriaLive?: string = 'assertive'; + /** (optional) string prepended to the heading */ + @Prop() innerRole?: 'alert' | 'status' = 'alert'; /** (optional) Label for close button */ @Prop() closeButtonLabel?: string = 'Close'; /** (optional) `title` for close button */ @@ -69,13 +71,13 @@ export class Notification { @Prop() headingLevel: number = 2; /** (optional) string prepended to the heading */ @Prop() ariaHeading?: string = 'Information'; + /** (optional) Injected styles */ @Prop() styles?: string; /** What actually triggers opening/closing the notification */ @State() isOpen: boolean = this.opened || false; @State() animationState: 'in' | 'out' | undefined; - @State() role: string = 'alert'; @State() hasTextSlot: boolean = false; // @State() hasActionSlot: boolean = false; // unused for now @@ -91,8 +93,9 @@ export class Notification { connectedCallback() { if (this.hostElement.hasAttribute('opened')) { - // Do not use `role="alert"` if opened/visible on page load - this.role = undefined; + if (this.innerAriaLive === 'polite' || this.innerRole === undefined || this.innerRole === 'status') { + this.innerRole = 'status'; + } this.isOpen = true; } if (this.delay !== undefined) { @@ -115,7 +118,6 @@ export class Notification { open = () => { this.isOpen = true; - this.role = 'alert'; this.animationState = 'in'; requestAnimationFrame(async () => { await animationsFinished(this.hostElement.shadowRoot); @@ -164,7 +166,7 @@ export class Notification { `variant-${this.variant}`, this.isOpen && 'open' )} - role={this.role} + role={this.innerRole} >
Date: Fri, 20 Oct 2023 16:14:08 +0200 Subject: [PATCH 2/9] style: format --- .../src/components/notification/notification.tsx | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/packages/components/src/components/notification/notification.tsx b/packages/components/src/components/notification/notification.tsx index 9084499aa3..29d37103a7 100644 --- a/packages/components/src/components/notification/notification.tsx +++ b/packages/components/src/components/notification/notification.tsx @@ -62,7 +62,7 @@ export class Notification { /** @deprecated - ariaRole should replace innerAriaLive */ @Prop() innerAriaLive?: string = 'assertive'; /** (optional) string prepended to the heading */ - @Prop() innerRole?: 'alert' | 'status' = 'alert'; + @Prop() innerRole?: 'alert' | 'status' = 'alert'; /** (optional) Label for close button */ @Prop() closeButtonLabel?: string = 'Close'; /** (optional) `title` for close button */ @@ -93,7 +93,11 @@ export class Notification { connectedCallback() { if (this.hostElement.hasAttribute('opened')) { - if (this.innerAriaLive === 'polite' || this.innerRole === undefined || this.innerRole === 'status') { + if ( + this.innerAriaLive === 'polite' || + this.innerRole === undefined || + this.innerRole === 'status' + ) { this.innerRole = 'status'; } this.isOpen = true; @@ -173,9 +177,7 @@ export class Notification {
-
+
Date: Mon, 23 Oct 2023 16:27:30 +0200 Subject: [PATCH 3/9] fix: use slot for heading, implement a11y workaround for role=status --- .../components/notification/notification.css | 4 +++- .../components/notification/notification.tsx | 17 ++++++++++++++++- 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/packages/components/src/components/notification/notification.css b/packages/components/src/components/notification/notification.css index 3dd3eba13c..3a33e2e36f 100644 --- a/packages/components/src/components/notification/notification.css +++ b/packages/components/src/components/notification/notification.css @@ -127,9 +127,11 @@ padding-left: var(--spacing-y); } -[part='heading'] { +[part='heading'], +[part='heading'] ::slotted(*) { font: var(--telekom-text-style-heading-6); line-height: var(--telekom-typography-line-spacing-tight); + margin: 0; } [part='text'] { diff --git a/packages/components/src/components/notification/notification.tsx b/packages/components/src/components/notification/notification.tsx index 29d37103a7..f6c19808a7 100644 --- a/packages/components/src/components/notification/notification.tsx +++ b/packages/components/src/components/notification/notification.tsx @@ -115,6 +115,8 @@ export class Notification { if (newValue === true) { this.open(); this.lastCloseEventTrigger = 'ATTRIBUTE'; + + } else if (this.isOpen) { this.close(); } @@ -131,6 +133,15 @@ export class Notification { setTimeout(this.timeout, this.delay); } }); + // workaround: when role is status, the inner text needs to be changed in order to be announced by the screen reader correctly + if (this.innerRole === "status") { + const text = this.hostElement.querySelector('[slot="text"]').innerHTML; + const heading = this.hostElement.querySelector('[slot="heading"]').innerHTML; + setTimeout( () => { + this.hostElement.querySelector('[slot="text"]').innerHTML = text; + this.hostElement.querySelector('[slot="heading"]').innerHTML = heading; + }, 10) + } }; close = () => { @@ -177,7 +188,10 @@ export class Notification {
-
+
{this.heading} +
{this.hasTextSlot && (
From 0faf96bfb991a4b0956f0858e6de5efb7b7b2881 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Mon, 30 Oct 2023 14:35:24 +0100 Subject: [PATCH 4/9] test(visual): update snapshots (#2172) Co-authored-by: felix-ico --- .../src/components/notification/readme.md | 29 ++++++++++--------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/packages/components/src/components/notification/readme.md b/packages/components/src/components/notification/readme.md index c887f7a336..c3fe2f6412 100644 --- a/packages/components/src/components/notification/readme.md +++ b/packages/components/src/components/notification/readme.md @@ -7,20 +7,21 @@ ## Properties -| Property | Attribute | Description | Type | Default | -| ------------------ | -------------------- | --------------------------------------------------------- | ------------------------------------------------------- | ----------------- | -| `ariaHeading` | `aria-heading` | (optional) string prepended to the heading | `string` | `'Information'` | -| `closeButtonLabel` | `close-button-label` | (optional) Label for close button | `string` | `'Close'` | -| `closeButtonTitle` | `close-button-title` | (optional) `title` for close button | `string` | `'Close'` | -| `delay` | `delay` | (optional) Time in milliseconds until it closes by itself | `number` | `undefined` | -| `dismissible` | `dismissible` | (optional) Show the close button | `boolean` | `false` | -| `heading` | `heading` | Heading | `string` | `undefined` | -| `headingLevel` | `heading-level` | Default aria-level for heading | `number` | `2` | -| `innerAriaLive` | `inner-aria-live` | (optional) `aria-live` of element | `string` | `'assertive'` | -| `opened` | `opened` | (optional) Visible | `boolean` | `undefined` | -| `styles` | `styles` | (optional) Injected styles | `string` | `undefined` | -| `type` | `type` | (optional) Type | `"banner" \| "inline" \| "toast"` | `'inline'` | -| `variant` | `variant` | (optional) Variant | `"danger" \| "informational" \| "success" \| "warning"` | `'informational'` | +| Property | Attribute | Description | Type | Default | +| ------------------ | -------------------- | ------------------------------------------------------------------------------------------------- | ------------------------------------------------------- | ----------------- | +| `ariaHeading` | `aria-heading` | (optional) string prepended to the heading | `string` | `'Information'` | +| `closeButtonLabel` | `close-button-label` | (optional) Label for close button | `string` | `'Close'` | +| `closeButtonTitle` | `close-button-title` | (optional) `title` for close button | `string` | `'Close'` | +| `delay` | `delay` | (optional) Time in milliseconds until it closes by itself | `number` | `undefined` | +| `dismissible` | `dismissible` | (optional) Show the close button | `boolean` | `false` | +| `heading` | `heading` | Heading | `string` | `undefined` | +| `headingLevel` | `heading-level` | Default aria-level for heading | `number` | `2` | +| `innerAriaLive` | `inner-aria-live` | **[DEPRECATED]** - ariaRole should replace innerAriaLive

| `string` | `'assertive'` | +| `innerRole` | `inner-role` | (optional) string prepended to the heading | `"alert" \| "status"` | `'alert'` | +| `opened` | `opened` | (optional) Visible | `boolean` | `undefined` | +| `styles` | `styles` | (optional) Injected styles | `string` | `undefined` | +| `type` | `type` | (optional) Type | `"banner" \| "inline" \| "toast"` | `'inline'` | +| `variant` | `variant` | (optional) Variant | `"danger" \| "informational" \| "success" \| "warning"` | `'informational'` | ## Events From e81901b04876cb6b73dce6cacd4808f8965faf96 Mon Sep 17 00:00:00 2001 From: felixw Date: Mon, 30 Oct 2023 14:44:47 +0100 Subject: [PATCH 5/9] style: format --- .../src/components/notification/notification.tsx | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/packages/components/src/components/notification/notification.tsx b/packages/components/src/components/notification/notification.tsx index f6c19808a7..9d07de9ef8 100644 --- a/packages/components/src/components/notification/notification.tsx +++ b/packages/components/src/components/notification/notification.tsx @@ -115,8 +115,6 @@ export class Notification { if (newValue === true) { this.open(); this.lastCloseEventTrigger = 'ATTRIBUTE'; - - } else if (this.isOpen) { this.close(); } @@ -134,13 +132,14 @@ export class Notification { } }); // workaround: when role is status, the inner text needs to be changed in order to be announced by the screen reader correctly - if (this.innerRole === "status") { + if (this.innerRole === 'status') { const text = this.hostElement.querySelector('[slot="text"]').innerHTML; - const heading = this.hostElement.querySelector('[slot="heading"]').innerHTML; - setTimeout( () => { + const heading = + this.hostElement.querySelector('[slot="heading"]').innerHTML; + setTimeout(() => { this.hostElement.querySelector('[slot="text"]').innerHTML = text; this.hostElement.querySelector('[slot="heading"]').innerHTML = heading; - }, 10) + }, 10); } }; @@ -188,10 +187,7 @@ export class Notification {
-
+
Date: Fri, 3 Nov 2023 15:23:59 +0100 Subject: [PATCH 6/9] fix: a11y remove unnecessary custom workaround --- .../src/components/notification/notification.tsx | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/packages/components/src/components/notification/notification.tsx b/packages/components/src/components/notification/notification.tsx index 9d07de9ef8..954f105ca6 100644 --- a/packages/components/src/components/notification/notification.tsx +++ b/packages/components/src/components/notification/notification.tsx @@ -131,16 +131,6 @@ export class Notification { setTimeout(this.timeout, this.delay); } }); - // workaround: when role is status, the inner text needs to be changed in order to be announced by the screen reader correctly - if (this.innerRole === 'status') { - const text = this.hostElement.querySelector('[slot="text"]').innerHTML; - const heading = - this.hostElement.querySelector('[slot="heading"]').innerHTML; - setTimeout(() => { - this.hostElement.querySelector('[slot="text"]').innerHTML = text; - this.hostElement.querySelector('[slot="heading"]').innerHTML = heading; - }, 10); - } }; close = () => { From 7ee93be8f15f411cdb3c6bb18b5757a03791691f Mon Sep 17 00:00:00 2001 From: felixw Date: Tue, 7 Nov 2023 11:37:39 +0100 Subject: [PATCH 7/9] fix: implement change requests --- .../components/src/components/notification/notification.tsx | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/components/src/components/notification/notification.tsx b/packages/components/src/components/notification/notification.tsx index 954f105ca6..1d0d7caeff 100644 --- a/packages/components/src/components/notification/notification.tsx +++ b/packages/components/src/components/notification/notification.tsx @@ -95,7 +95,6 @@ export class Notification { if (this.hostElement.hasAttribute('opened')) { if ( this.innerAriaLive === 'polite' || - this.innerRole === undefined || this.innerRole === 'status' ) { this.innerRole = 'status'; @@ -177,14 +176,14 @@ export class Notification {
-
+
- {this.heading} + {this.heading ? {this.heading} : null}
{this.hasTextSlot && ( From 93d516909b732acbb88e3cfdefd8a6f1b999ad61 Mon Sep 17 00:00:00 2001 From: felixw Date: Wed, 22 Nov 2023 16:03:59 +0100 Subject: [PATCH 8/9] style: format --- .../components/src/components/notification/notification.tsx | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/packages/components/src/components/notification/notification.tsx b/packages/components/src/components/notification/notification.tsx index 1d0d7caeff..3cc19abe30 100644 --- a/packages/components/src/components/notification/notification.tsx +++ b/packages/components/src/components/notification/notification.tsx @@ -93,10 +93,7 @@ export class Notification { connectedCallback() { if (this.hostElement.hasAttribute('opened')) { - if ( - this.innerAriaLive === 'polite' || - this.innerRole === 'status' - ) { + if (this.innerAriaLive === 'polite' || this.innerRole === 'status') { this.innerRole = 'status'; } this.isOpen = true; From 6c42d60dfd3302bf0e36f7f2ebee0e4d7bbfb76c Mon Sep 17 00:00:00 2001 From: felixw Date: Fri, 24 Nov 2023 16:04:49 +0100 Subject: [PATCH 9/9] docs: document prop change --- .../stories/components/notification/Notification.stories.mdx | 3 ++- .../stories/components/notification/ScaleNotification.vue | 1 + 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/storybook-vue/stories/components/notification/Notification.stories.mdx b/packages/storybook-vue/stories/components/notification/Notification.stories.mdx index fe61a250fc..dc242dde7c 100644 --- a/packages/storybook-vue/stories/components/notification/Notification.stories.mdx +++ b/packages/storybook-vue/stories/components/notification/Notification.stories.mdx @@ -49,7 +49,8 @@ import ScaleNotification from './ScaleNotification.vue'; innerAriaLive: { control: { type: 'text' - } + }, + description: 'Deprecated! innerAriaLive has been replaced by innerRole' }, headingLevel: { control: { diff --git a/packages/storybook-vue/stories/components/notification/ScaleNotification.vue b/packages/storybook-vue/stories/components/notification/ScaleNotification.vue index 40b36ba336..1adb3b6d4b 100644 --- a/packages/storybook-vue/stories/components/notification/ScaleNotification.vue +++ b/packages/storybook-vue/stories/components/notification/ScaleNotification.vue @@ -14,6 +14,7 @@ export default { dismissible: { type: Boolean }, delay: { type: Number }, innerAriaLive: { type: String }, + innerRole: { type: String, default: 'alert' }, closeButtonLabel: { type: String }, closeButtonTitle: { type: String }, styles: { type: String },