Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

Commit

Permalink
fix(context-dialog): Fixes an issue with aria label name clashes for …
Browse files Browse the repository at this point in the history
…a11y.

Specific inputs like `aria-label-close-button` are not allowed based on
the aria specification. Only known aria attributes are allowed to be
used. We have renamed the inputs in question to their camelcased
version, but kept the old ones for compatibility for now.
With version 7.0. we sould remove the kebap-cased inputs.

Fixes #79
  • Loading branch information
tomheller committed Feb 6, 2020
1 parent 6776641 commit ec844b9
Show file tree
Hide file tree
Showing 21 changed files with 59 additions and 40 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
<section>
<dt-context-dialog
id="context-dialog"
aria-label-close-button="close"
ariaLabelClose="close"
[disabled]="disabled"
>
<p>Some awesome content</p>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<dt-context-dialog
aria-label="Open context dialog"
aria-label-close-button="Close context dialog"
ariaLabelClose="Close context dialog"
[overlayPanelClass]="panel"
>
<button dt-button>First button</button>
Expand Down
2 changes: 1 addition & 1 deletion apps/dev/src/menu/menu-demo.html
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
<dt-context-dialog
class="demo-menu-context-dialog"
aria-label="Show more details"
aria-label-close-button="Close context dialog"
ariaLabelClose="Close context dialog"
>
<dt-menu aria-label="Demo Menu" class="demo-menu">
<dt-menu-group label="Dashboards & reports">
Expand Down
16 changes: 8 additions & 8 deletions components/context-dialog/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,14 @@ class MyModule {}

## Inputs

| Name | Type | Default | Description |
| ------------------------- | ---------------------------------------------------------- | ----------- | ------------------------------------------------------------------------------------------------------- |
| `tabIndex` | `number` | `0` | Gets and sets the tabIndex on the context dialog. Inherited by mixinTabIndex. |
| `disabled` | `boolean` | `false` | Gets and sets the disabled property on the context dialog. Inherited by mixinDisabled. |
| `aria-label` | `string` | `undefined` | ARIA label of the context dialog trigger button. |
| `aria-labelledby` | `string` | `undefined` | ARIA reference to a label describing the context-dialog. |
| `aria-label-close-button` | `string` | | ARIA label of the context dialog close button. |
| `overlayPanelClass` | `string | string[] | Set<string> | { [key: string]: any }` | | Custom css classes to add to the overlay panel element. Can be used to scope styling within the overlay |
| Name | Type | Default | Description |
| ------------------- | ---------------------------------------------------------- | ----------- | ------------------------------------------------------------------------------------------------------- |
| `tabIndex` | `number` | `0` | Gets and sets the tabIndex on the context dialog. Inherited by mixinTabIndex. |
| `disabled` | `boolean` | `false` | Gets and sets the disabled property on the context dialog. Inherited by mixinDisabled. |
| `aria-label` | `string` | `undefined` | ARIA label of the context dialog trigger button. |
| `aria-labelledby` | `string` | `undefined` | ARIA reference to a label describing the context-dialog. |
| `ariaLabelClose` | `string` | | ARIA label of the context dialog close button. |
| `overlayPanelClass` | `string | string[] | Set<string> | { [key: string]: any }` | | Custom css classes to add to the overlay panel element. Can be used to scope styling within the overlay |

To make our components accessible it is obligatory to provide either an
`aria-label` or `aria-labelledby`.
Expand Down
2 changes: 1 addition & 1 deletion components/context-dialog/src/context-dialog.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -366,7 +366,7 @@ describe('DtContextDialog', () => {
<dt-context-dialog
#dialog
[aria-label]="ariaLabel"
[aria-label-close-button]="ariaLabelClose"
[ariaLabelClose]="ariaLabelClose"
[tabIndex]="tabIndexOverride"
[disabled]="disabled"
[overlayPanelClass]="panelClass"
Expand Down
24 changes: 21 additions & 3 deletions components/context-dialog/src/context-dialog.ts
Original file line number Diff line number Diff line change
Expand Up @@ -149,10 +149,28 @@ export class DtContextDialog extends _DtContextDialogMixinBase

/**
* Aria label of the context-dialog's close button.
* @deprecated Made obsolete without the closing button
* @breaking-change To be removed with 6.0.0.
* @deprecated `aria-label-close-button` does not conform with accessibility standards.
* Please use `ariaLabelClose` input instead.
* @breaking-change Will be removed in version 7.0.0
*/
@Input('aria-label-close-button') ariaLabelClose: string;
@Input('aria-label-close-button')
get depAriaLabelClose(): string {
return this._ariaLabelClose;
}
set depAriaLabelClose(value: string) {
this._ariaLabelClose = value;
}

/** Aria label of the context-dialog's close button. */
@Input()
get ariaLabelClose(): string {
return this._ariaLabelClose;
}
set ariaLabelClose(value: string) {
this._ariaLabelClose = value;
}
/** @internal Aria label of the context-dialog's close button. */
_ariaLabelClose: string;

/** The custom class to add to the overlay panel element. Can be used to scope styling within the overlay */
@Input() overlayPanelClass:
Expand Down
2 changes: 1 addition & 1 deletion documentation/linting.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ preventing those errors:
| `dt-card-no-empty` | A card must contain content apart from the predefined child components `dt-card-title`, `dt-card-subtitle`, `dt-card-icon`, `dt-card-title-actions` and `dt-card-footer-actions`. |
| `dt-checkbox-no-empty` | A checkbox must always contain text content or a component that renders text. If no content is given, an `aria-label` or `aria-labelledby` attribute is required. |
| `dt-consumption-icon-alt-text` | A `dt-consumption-icom` must always have an alternative text in form of an `aria-label` or an `aria-labelledby` attribute. |
| `dt-context-dialog-alt-text` | The open and the close button need additional attributes to provide text alternatives using the following inputs: `aria-label` and `aria-label-close-button` |
| `dt-context-dialog-alt-text` | The open and the close button need additional attributes to provide text alternatives using the following inputs: `aria-label` and `ariaLabelClose` |
| `dt-copy-to-clipboard-no-empty` | The copy-to-clipboard component must always contain a `dt-copy-to-clipboard-label`, that is a direct child of `dt-copy-to-clipboard`. |
| `dt-empty-state-requires-item` | The `<dt-empty-state>` must contain at least one `<dt-empty-state-item>`. |
| `dt-expandable-trigger-is-button` | The trigger of an expandable panel must always be a button element. |
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
<dt-card-title-actions>
<dt-context-dialog
aria-label="Show more actions"
aria-label-close-button="Close context dialog"
ariaLabelClose="Close context dialog"
>
<button dt-button variant="secondary">First button</button>
<button dt-button variant="secondary">Second button</button>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
<dt-context-dialog
#dialog
aria-label="Open context dialog"
aria-label-close-button="Close context dialog"
ariaLabelClose="Close context dialog"
>
<p>
Your dashboard "real user monitoring"
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<div class="dt-example-dark" dtTheme=":dark">
<dt-context-dialog
aria-label="Show more actions"
aria-label-close-button="Close context dialog"
ariaLabelClose="Close context dialog"
>
<button dt-button variant="secondary">Edit</button>
</dt-context-dialog>
Expand All @@ -16,7 +16,7 @@
<dt-context-dialog
#darkIcondialog
aria-label="Open context dialog"
aria-label-close-button="Close context dialog"
ariaLabelClose="Close context dialog"
>
<p>
Your dashboard "real user monitoring"
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<dt-context-dialog
aria-label="Show more details"
aria-label-close-button="Close context dialog"
ariaLabelClose="Close context dialog"
>
<p>Your dashboard "real user monitoring" is only visible to you</p>
</dt-context-dialog>
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<dt-context-dialog
aria-label="Show more actions"
aria-label-close-button="Close context dialog"
ariaLabelClose="Close context dialog"
>
<dt-context-dialog-header>
<dt-context-dialog-header-title>Analyse</dt-context-dialog-header-title>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
#interactiveDialog
[disabled]="interactiveDialogDisabled"
aria-label="Show more details"
aria-label-close-button="Close context dialog"
ariaLabelClose="Close context dialog"
>
<p>
Your dashboard "real user monitoring"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
<dt-context-dialog
color="cta"
aria-label="Show more details"
aria-label-close-button="Close context dialog"
ariaLabelClose="Close context dialog"
>
<p>Close me to return the focus to the "Open" button</p>
<button dt-button variant="secondary">Focused</button>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
#contextdialog
color="cta"
aria-label="Show more actions"
aria-label-close-button="Close context dialog"
ariaLabelClose="Close context dialog"
>
<dt-copy-to-clipboard (afterCopy)="contextdialog.close()">
<input
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<dt-context-dialog
aria-label="Show more details"
aria-label-close-button="Close context dialog"
ariaLabelClose="Close context dialog"
>
<dt-menu aria-label="Menu inside Popup Example">
<dt-menu-group label="Dashboards & reports">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@
<dt-context-dialog
#dialog
aria-label="Show more data"
aria-label-close-button="Close context dialog"
ariaLabelClose="Close context dialog"
>
{{ row.name }} context dialog
</dt-context-dialog>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@
<dt-context-dialog
#dialog
aria-label="Show more data"
aria-label-close-button="Close context dialog"
ariaLabelClose="Close context dialog"
>
{{ row.name }} context dialog
</dt-context-dialog>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@
<dt-context-dialog
#dialog
aria-label="Show more data"
aria-label-close-button="Close context dialog"
ariaLabelClose="Close context dialog"
>
{{ row.name }} context dialog
</dt-context-dialog>
Expand Down
7 changes: 4 additions & 3 deletions tools/linting/src/rules/dtContextDialogAltTextRule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,15 +62,16 @@ class DtContextDialogVisitor extends BasicTemplateAstVisitor {
if (isElementWithName(element, 'dt-context-dialog')) {
if (
hasTextContentAlternative(element, 'aria-label') &&
hasTextContentAlternative(element, 'aria-label-close-button')
(hasTextContentAlternative(element, 'aria-label-close-button') ||
hasTextContentAlternative(element, 'ariaLabelClose'))
) {
return;
}

addFailure(
this,
element,
'A context dialog must provide alternative texts for the open and the close buttons. Use the aria-label and the aria-label-close-button input.',
'A context dialog must provide alternative texts for the open and the close buttons. Use the aria-label and the ariaLabelClose input.',
);
}
}
Expand All @@ -81,7 +82,7 @@ class DtContextDialogVisitor extends BasicTemplateAstVisitor {
* the open and the close button of a context dialog.
*
* The following example passes the lint checks:
* <dt-context-dialog aria-label="Open more options" aria-label-close-button="Close context dialog">
* <dt-context-dialog aria-label="Open more options" ariaLabelClose="Close context dialog">
* <p>Your dashboard "real user monitoring" is only visible to you</p>
* </dt-context-dialog>
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,45 +5,45 @@

@Component({
template: `
<dt-context-dialog aria-label="Open more options" aria-label-close-button="Close context dialog">
<dt-context-dialog aria-label="Open more options" ariaLabelClose="Close context dialog">
<p>Your dashboard "real user monitoring" is only visible to you</p>
</dt-context-dialog>

<dt-context-dialog aria-label-close-button="Close context dialog">
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [A context dialog must provide alternative texts for the open and the close buttons. Use the aria-label and the aria-label-close-button input.]
<dt-context-dialog ariaLabelClose="Close context dialog">
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [A context dialog must provide alternative texts for the open and the close buttons. Use the aria-label and the ariaLabelClose input.]
<p>Your dashboard "real user monitoring" is only visible to you</p>
</dt-context-dialog>

<dt-context-dialog aria-label="Open more options">
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [A context dialog must provide alternative texts for the open and the close buttons. Use the aria-label and the aria-label-close-button input.]
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [A context dialog must provide alternative texts for the open and the close buttons. Use the aria-label and the ariaLabelClose input.]
<p>Your dashboard "real user monitoring" is only visible to you</p>
</dt-context-dialog>

<dt-context-dialog>
~~~~~~~~~~~~~~~~~~~ [A context dialog must provide alternative texts for the open and the close buttons. Use the aria-label and the aria-label-close-button input.]
~~~~~~~~~~~~~~~~~~~ [A context dialog must provide alternative texts for the open and the close buttons. Use the aria-label and the ariaLabelClose input.]
<p>Your dashboard "real user monitoring" is only visible to you</p>
</dt-context-dialog>

<button dt-icon-button [dtContextDialogTrigger]="dialog" aria-label="Open context dialog">
<dt-icon name="agent"></dt-icon>
</button>
<dt-context-dialog #dialog aria-label="Open context dialog" aria-label-close-button="Close context dialog">
<dt-context-dialog #dialog aria-label="Open context dialog" ariaLabelClose="Close context dialog">
<p>Your dashboard "real user monitoring"<br> is only visible to you</p>
</dt-context-dialog>

<button dt-icon-button [dtContextDialogTrigger]="dialog" aria-labelledby="description">
<dt-icon name="agent"></dt-icon>
</button>
<p id="description">Click button to open the context dialog.</p>
<dt-context-dialog #dialog aria-label="Open context dialog" aria-label-close-button="Close context dialog">
<dt-context-dialog #dialog aria-label="Open context dialog" ariaLabelClose="Close context dialog">
<p>Your dashboard "real user monitoring"<br> is only visible to you</p>
</dt-context-dialog>

<button dt-icon-button [dtContextDialogTrigger]="dialog">
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [A context dialog trigger must have an aria-label or aria-labelledby attribute.]
<dt-icon name="agent"></dt-icon>
</button>
<dt-context-dialog #dialog aria-label="Open context dialog" aria-label-close-button="Close context dialog">
<dt-context-dialog #dialog aria-label="Open context dialog" ariaLabelClose="Close context dialog">
<p>Your dashboard "real user monitoring"<br> is only visible to you</p>
</dt-context-dialog>
`,
Expand Down

0 comments on commit ec844b9

Please sign in to comment.