Skip to content

Commit

Permalink
fix(modal): ariaLabel and role are inherited when set via htmlAttribu…
Browse files Browse the repository at this point in the history
…tes (#29099)

Issue number: Internal

---------

<!-- Please do not submit updates to dependencies unless it fixes an
issue. -->

<!-- Please try to limit your pull request to one type (bugfix, feature,
etc). Submit multiple pull requests if needed. -->

## What is the current behavior?
<!-- Please describe the current behavior that you are modifying. -->

Modal inherits `aria-label` and `role` to an element inside of its
Shadow DOM. However, this only works if developers set the attributes on
the host element directly. Setting the attributes via the
`htmlAttributes` property causes the attributes to be set on the host
and not inherited.

## What is the new behavior?
<!-- Please describe the behavior or changes that are being added by
this PR. -->

- `aria-label` and `role` are inherited even when set using
`htmlAttributes`.

## Does this introduce a breaking change?

- [ ] Yes
- [x] No

<!--
  If this introduces a breaking change:
1. Describe the impact and migration path for existing applications
below.
  2. Update the BREAKING.md file with the breaking change.
3. Add "BREAKING CHANGE: [...]" to the commit description when merging.
See
https://github.com/ionic-team/ionic-framework/blob/main/.github/CONTRIBUTING.md#footer
for more information.
-->


## Other information

<!-- Any other information that is important to this PR such as
screenshots of how the component looks before and after the change. -->

Dev build: `7.7.4-dev.11709154993.1b49c313`
  • Loading branch information
liamdebeasi authored Feb 29, 2024
1 parent 5a5f330 commit de13633
Show file tree
Hide file tree
Showing 3 changed files with 78 additions and 25 deletions.
41 changes: 39 additions & 2 deletions core/src/components/modal/modal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -345,10 +345,47 @@ export class Modal implements ComponentInterface, OverlayInterface {
}

componentWillLoad() {
const { breakpoints, initialBreakpoint, el } = this;
const { breakpoints, initialBreakpoint, el, htmlAttributes } = this;
const isSheetModal = (this.isSheetModal = breakpoints !== undefined && initialBreakpoint !== undefined);

this.inheritedAttributes = inheritAttributes(el, ['aria-label', 'role']);
const attributesToInherit = ['aria-label', 'role'];
this.inheritedAttributes = inheritAttributes(el, attributesToInherit);

/**
* When using a controller modal you can set attributes
* using the htmlAttributes property. Since the above attributes
* need to be inherited inside of the modal, we need to look
* and see if these attributes are being set via htmlAttributes.
*
* We could alternatively move this to componentDidLoad to simplify the work
* here, but we'd then need to make inheritedAttributes a State variable,
* thus causing another render to always happen after the first render.
*/
if (htmlAttributes !== undefined) {
attributesToInherit.forEach((attribute) => {
const attributeValue = htmlAttributes[attribute];
if (attributeValue) {
/**
* If an attribute we need to inherit was
* set using htmlAttributes then add it to
* inheritedAttributes and remove it from htmlAttributes.
* This ensures the attribute is inherited and not
* set on the host.
*
* In this case, if an inherited attribute is set
* on the host element and using htmlAttributes then
* htmlAttributes wins, but that's not a pattern that we recommend.
* The only time you'd need htmlAttributes is when using modalController.
*/
this.inheritedAttributes = {
...this.inheritedAttributes,
[attribute]: htmlAttributes[attribute],
};

delete htmlAttributes[attribute];
}
});
}

if (isSheetModal) {
this.currentBreakpoint = this.initialBreakpoint;
Expand Down
23 changes: 0 additions & 23 deletions core/src/components/modal/test/a11y/modal.spec.ts

This file was deleted.

39 changes: 39 additions & 0 deletions core/src/components/modal/test/modal-attributes.spec.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
import { h } from '@stencil/core';
import { newSpecPage } from '@stencil/core/testing';

import { Modal } from '../modal';

it('should inherit attributes', async () => {
/**
* Note: This example should not be used in production.
* This only serves to check that `role` can be customized.
*/
const page = await newSpecPage({
components: [Modal],
template: () => <ion-modal overlayIndex={1} aria-label="my label" role="presentation"></ion-modal>,
});

const modal = page.body.querySelector('ion-modal')!;
const contentWrapper = modal.shadowRoot!.querySelector('[part="content"]')!;

expect(contentWrapper.getAttribute('aria-label')).toBe('my label');
expect(contentWrapper.getAttribute('role')).toBe('presentation');
});

it('should inherit attributes when set via htmlAttributes', async () => {
const page = await newSpecPage({
components: [Modal],
template: () => (
<ion-modal overlayIndex={1} htmlAttributes={{ 'aria-label': 'my label', role: 'presentation' }}></ion-modal>
),
});

const modal = page.body.querySelector('ion-modal')!;
const contentWrapper = modal.shadowRoot!.querySelector('[part="content"]')!;

expect(contentWrapper.getAttribute('aria-label')).toBe('my label');
expect(contentWrapper.getAttribute('role')).toBe('presentation');

expect(modal.hasAttribute('aria-label')).toBe(false);
expect(modal.hasAttribute('role')).toBe(false);
});

0 comments on commit de13633

Please sign in to comment.