diff --git a/.github/workflows/actions/test-angular-package/action.yml b/.github/workflows/actions/test-angular-package/action.yml new file mode 100644 index 00000000000..4f6181a2115 --- /dev/null +++ b/.github/workflows/actions/test-angular-package/action.yml @@ -0,0 +1,26 @@ +name: 'Test Ionic Angular Package' +description: 'Test Ionic Angular Package' +runs: + using: 'composite' + steps: + - uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7.0.0 + with: + node-version: 24.x + - uses: ./.github/workflows/actions/download-archive + with: + name: ionic-core + path: ./core + filename: CoreBuild.zip + - uses: ./.github/workflows/actions/download-archive + with: + name: ionic-angular + path: ./packages/angular + filename: AngularBuild.zip + - name: πŸ•ΈοΈ Install Angular Dependencies + run: npm ci + shell: bash + working-directory: ./packages/angular + - name: πŸ“ Run Angular Package Tests + run: npm run test + shell: bash + working-directory: ./packages/angular diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 683f9de2004..3e18b7e35b1 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -129,6 +129,13 @@ jobs: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - uses: ./.github/workflows/actions/build-angular + test-angular-package: + needs: [build-angular] + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + - uses: ./.github/workflows/actions/test-angular-package + build-angular-server: needs: [build-core] runs-on: ubuntu-latest diff --git a/.github/workflows/codeql-analysis.yml b/.github/workflows/codeql-analysis.yml index df238d50502..9fa6a37afb7 100644 --- a/.github/workflows/codeql-analysis.yml +++ b/.github/workflows/codeql-analysis.yml @@ -15,7 +15,7 @@ jobs: security-events: write steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - - uses: github/codeql-action/init@cdf488f595d80d6e07e03d4674febd5ab45fa938 # v4.37.9 + - uses: github/codeql-action/init@1c5b675653bb5c22dbe9b12b556ec555138e09fd # v4.38.1 with: languages: javascript - - uses: github/codeql-action/analyze@cdf488f595d80d6e07e03d4674febd5ab45fa938 # v4.37.9 + - uses: github/codeql-action/analyze@1c5b675653bb5c22dbe9b12b556ec555138e09fd # v4.38.1 diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index bb552039012..5536a4e8923 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -85,9 +85,17 @@ jobs: # possible for them to push at the same time. needs: [finalize-release] runs-on: ubuntu-latest + # This is a backstop for the retry loop in + # Resolve Package Locks, which fails with its + # own message first. + timeout-minutes: 30 permissions: contents: write id-token: write + outputs: + # This says the new versions resolved on npm, + # which is not the same as the job passing. + resolved: ${{ steps.resolve.outputs.resolved }} steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 # Pull the latest version of the reference @@ -108,25 +116,74 @@ jobs: git config user.email hi@ionicframework.com shell: bash # Lerna does not automatically bump versions - # of Ionic dependencies that have changed, - # so we do that here. - - name: Bump Package Lock + # of Ionic dependencies that have changed, so + # we do that here. The install fails with + # ETARGET until the versions published earlier + # resolve on npm, so retry. The + # `--prefer-online` flag matters because npm + # caches the failed lookup, and an `npm view` + # probe would read a different cached document. + - name: Resolve Package Locks + id: resolve + env: + SLEEP_SECONDS: 15 + DEADLINE_SECONDS: 900 + run: | + set -euo pipefail + # We bound this on the wall clock so the loop reports its own + # error before `timeout-minutes` kills the job. The `timeout` + # call caps an attempt that hangs rather than fails, which would + # never reach the check below. + deadline=$(( SECONDS + DEADLINE_SECONDS )) + attempt=0 + until timeout --kill-after=30 300 lerna exec "npm install --package-lock-only --prefer-online"; do + attempt=$(( attempt + 1 )) + if [ "$SECONDS" -ge "$deadline" ]; then + echo "::error::npm install --package-lock-only failed $attempt times over ${SECONDS}s; see the last attempt above for the error" + exit 1 + fi + echo "Attempt $attempt failed, retrying in ${SLEEP_SECONDS}s" + sleep "$SLEEP_SECONDS" + done + echo "resolved=true" >> "$GITHUB_OUTPUT" + shell: bash + # This is split from the retry so `resolved` + # does not depend on the commit and the push + # working. + - name: Commit Package Locks + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} run: | - lerna exec "npm install --package-lock-only" + set -euo pipefail git add . + # Re-running this job after the locks were already committed + # would otherwise fail on an empty commit. + if git diff --cached --quiet; then + echo "No package lock changes to commit" + exit 0 + fi git commit -m "chore(): update package lock files" git push - env: - GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} shell: bash purge-cdn-cache: - needs: [release-ionic] + # This runs after `update-package-lock` so the + # new versions have resolved on npm. Purging + # earlier re-caches `@ionic/core@latest` at the + # previous release. A failed `finalize-release` + # skips this too, so a half-finished release + # that did reach npm needs a manual purge. + needs: [update-package-lock] + if: ${{ !cancelled() && needs.update-package-lock.outputs.resolved == 'true' }} runs-on: ubuntu-latest + timeout-minutes: 5 steps: - name: Purge JSDelivr Cache run: | - curl -X POST \ + set -euo pipefail + if ! response=$(curl -sS --max-time 60 --retry 3 --retry-all-errors \ + --retry-max-time 120 \ + -w '\n%{http_code}' -X POST \ https://purge.jsdelivr.net/ \ -H 'cache-control: no-cache' \ -H 'content-type: application/json' \ @@ -142,7 +199,21 @@ jobs: "/npm/@ionic/core@7/css/ionic.bundle.css", "/npm/@ionic/core@8/css/ionic.bundle.css", "/npm/@ionic/core@9/css/ionic.bundle.css", - "/npm/@ionic/core@latest/css/ionic.bundle.css" + "/npm/@ionic/core@latest/css/ionic.bundle.css", "/npm/@ionic/core@next/css/ionic.bundle.css" - ]}' + ]}'); then + echo "::warning::jsDelivr purge request failed" + exit 0 + fi + status=${response##*$'\n'} + echo "${response%$'\n'*}" + # `curl` exits 0 whatever jsDelivr answers, so check the status. + # A 4xx means we sent a bad payload, which is otherwise silent. + # Anything else is jsDelivr being down, which should not fail + # a release. + case "$status" in + 2*) ;; + 4*) echo "::error::jsDelivr rejected the purge with HTTP $status"; exit 1 ;; + *) echo "::warning::jsDelivr purge did not complete (HTTP $status)" ;; + esac shell: bash diff --git a/CHANGELOG.md b/CHANGELOG.md index 0bbb6b6bdd3..f27c8b343bd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,33 @@ All notable changes to this project will be documented in this file. See [Conventional Commits](https://conventionalcommits.org) for commit guidelines. +## [9.0.5](https://github.com/ionic-team/ionic-framework/compare/v9.0.4...v9.0.5) (2026-09-23) + +### Bug Fixes + +* **button:** sync aria attributes between host and native button ([#31264](https://github.com/ionic-team/ionic-framework/issues/31264)) ([e771519](https://github.com/ionic-team/ionic-framework/commit/e771519373e787af0ad83611932ee113bc372a6e)), closes [#30626](https://github.com/ionic-team/ionic-framework/issues/30626) +* **datetime:** tear down ready state only when the host is hidden ([#31460](https://github.com/ionic-team/ionic-framework/issues/31460)) ([bf0607e](https://github.com/ionic-team/ionic-framework/commit/bf0607eece4703fc0d812e59a3a9b2c87fce7b09)), closes [#30933](https://github.com/ionic-team/ionic-framework/issues/30933) [#31108](https://github.com/ionic-team/ionic-framework/issues/31108) +* **popover:** account for CSS zoom in positioning and sizing ([#31426](https://github.com/ionic-team/ionic-framework/issues/31426)) ([96ff7df](https://github.com/ionic-team/ionic-framework/commit/96ff7dff6d7a1c449b7ac2c5e0fc98f21f5f134e)), closes [#30919](https://github.com/ionic-team/ionic-framework/issues/30919) [#31047](https://github.com/ionic-team/ionic-framework/issues/31047) [floating-ui/floating-ui#3492](https://github.com/floating-ui/floating-ui/issues/3492) +* **vue:** respect config log level ([#31452](https://github.com/ionic-team/ionic-framework/issues/31452)) ([edb3e48](https://github.com/ionic-team/ionic-framework/commit/edb3e48e28f3829da6a2a6ea436978b9ab92766d)) + + +## [9.0.4](https://github.com/ionic-team/ionic-framework/compare/v9.0.3...v9.0.4) (2026-09-16) + +### Bug Fixes + +* **input, select, textarea:** emit one click event when slotted content is clicked ([#31423](https://github.com/ionic-team/ionic-framework/issues/31423)) ([d6acf12](https://github.com/ionic-team/ionic-framework/commit/d6acf12477d1f6633580a10b4655c5ddb382844f)) +* **input, textarea:** keep the value visible when slotted content is wide ([#31435](https://github.com/ionic-team/ionic-framework/issues/31435)) ([1fb47c5](https://github.com/ionic-team/ionic-framework/commit/1fb47c55fbc590ef38b0b9600d1287be7a2177f1)) +* **modal:** prevent ion-content collapsing at content-based heights ([#31413](https://github.com/ionic-team/ionic-framework/issues/31413)) ([8a713ab](https://github.com/ionic-team/ionic-framework/commit/8a713ab84e4a4fdfe2eba68142cfd65ec1fd13b4)), closes [#31149](https://github.com/ionic-team/ionic-framework/issues/31149) + + +## [9.0.3](https://github.com/ionic-team/ionic-framework/compare/v9.0.2...v9.0.3) (2026-09-09) + +### Bug Fixes + +* **vue-router:** clear navigation info when a guard aborts navigation ([#31364](https://github.com/ionic-team/ionic-framework/issues/31364)) ([8d41b5f](https://github.com/ionic-team/ionic-framework/commit/8d41b5fff1f36b33ba3ccbd1bbf93a728ace3de6)), closes [#29721](https://github.com/ionic-team/ionic-framework/issues/29721) +* **vue:** support attribute autocomplete in WebStorm ([#31419](https://github.com/ionic-team/ionic-framework/issues/31419)) ([49d6339](https://github.com/ionic-team/ionic-framework/commit/49d6339644b71a3289e34650687c1f99b3ee21f4)) + + ## [9.0.2](https://github.com/ionic-team/ionic-framework/compare/v9.0.1...v9.0.2) (2026-09-02) ### Bug Fixes diff --git a/core/CHANGELOG.md b/core/CHANGELOG.md index 35b3789674f..d674ede539d 100644 --- a/core/CHANGELOG.md +++ b/core/CHANGELOG.md @@ -3,6 +3,33 @@ All notable changes to this project will be documented in this file. See [Conventional Commits](https://conventionalcommits.org) for commit guidelines. +## [9.0.5](https://github.com/ionic-team/ionic-framework/compare/v9.0.4...v9.0.5) (2026-09-23) + +### Bug Fixes + +* **button:** sync aria attributes between host and native button ([#31264](https://github.com/ionic-team/ionic-framework/issues/31264)) ([e771519](https://github.com/ionic-team/ionic-framework/commit/e771519373e787af0ad83611932ee113bc372a6e)), closes [#30626](https://github.com/ionic-team/ionic-framework/issues/30626) +* **datetime:** tear down ready state only when the host is hidden ([#31460](https://github.com/ionic-team/ionic-framework/issues/31460)) ([bf0607e](https://github.com/ionic-team/ionic-framework/commit/bf0607eece4703fc0d812e59a3a9b2c87fce7b09)), closes [#30933](https://github.com/ionic-team/ionic-framework/issues/30933) [#31108](https://github.com/ionic-team/ionic-framework/issues/31108) +* **popover:** account for CSS zoom in positioning and sizing ([#31426](https://github.com/ionic-team/ionic-framework/issues/31426)) ([96ff7df](https://github.com/ionic-team/ionic-framework/commit/96ff7dff6d7a1c449b7ac2c5e0fc98f21f5f134e)), closes [#30919](https://github.com/ionic-team/ionic-framework/issues/30919) [#31047](https://github.com/ionic-team/ionic-framework/issues/31047) [floating-ui/floating-ui#3492](https://github.com/floating-ui/floating-ui/issues/3492) +* **vue:** respect config log level ([#31452](https://github.com/ionic-team/ionic-framework/issues/31452)) ([edb3e48](https://github.com/ionic-team/ionic-framework/commit/edb3e48e28f3829da6a2a6ea436978b9ab92766d)) + + +## [9.0.4](https://github.com/ionic-team/ionic-framework/compare/v9.0.3...v9.0.4) (2026-09-16) + +### Bug Fixes + +* **input, select, textarea:** emit one click event when slotted content is clicked ([#31423](https://github.com/ionic-team/ionic-framework/issues/31423)) ([d6acf12](https://github.com/ionic-team/ionic-framework/commit/d6acf12477d1f6633580a10b4655c5ddb382844f)) +* **input, textarea:** keep the value visible when slotted content is wide ([#31435](https://github.com/ionic-team/ionic-framework/issues/31435)) ([1fb47c5](https://github.com/ionic-team/ionic-framework/commit/1fb47c55fbc590ef38b0b9600d1287be7a2177f1)) +* **modal:** prevent ion-content collapsing at content-based heights ([#31413](https://github.com/ionic-team/ionic-framework/issues/31413)) ([8a713ab](https://github.com/ionic-team/ionic-framework/commit/8a713ab84e4a4fdfe2eba68142cfd65ec1fd13b4)), closes [#31149](https://github.com/ionic-team/ionic-framework/issues/31149) + + +## [9.0.3](https://github.com/ionic-team/ionic-framework/compare/v9.0.2...v9.0.3) (2026-09-09) + +**Note:** Version bump only for package @ionic/core + + + + + ## [9.0.2](https://github.com/ionic-team/ionic-framework/compare/v9.0.1...v9.0.2) (2026-09-02) ### Bug Fixes diff --git a/core/package-lock.json b/core/package-lock.json index e7abbf995eb..babea9f4ec1 100644 --- a/core/package-lock.json +++ b/core/package-lock.json @@ -1,12 +1,12 @@ { "name": "@ionic/core", - "version": "9.0.2", + "version": "9.0.5", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@ionic/core", - "version": "9.0.2", + "version": "9.0.5", "license": "MIT", "dependencies": { "@stencil/core": "^4.44.2", @@ -733,9 +733,9 @@ } }, "node_modules/@capacitor/core": { - "version": "8.5.1", - "resolved": "https://registry.npmjs.org/@capacitor/core/-/core-8.5.1.tgz", - "integrity": "sha512-IOQ7ynG1qgrJ/RLuCO3WgyRGmkmmG/cdStRR4ftU2dWEN1FdtLrnI/XpZmZuUsOEHEnzBRJY/PmRH8eTLiXfhA==", + "version": "8.5.2", + "resolved": "https://registry.npmjs.org/@capacitor/core/-/core-8.5.2.tgz", + "integrity": "sha512-agnQbGdhGw/KZFuLf7H9SY2o+oE7upkr/WPQsYaokXP6f9chaE8iVmbKkfN6SVl4w3pmz7oZ3Ur1hbkmy1hx+w==", "dev": true, "license": "MIT", "dependencies": { @@ -2380,9 +2380,9 @@ } }, "node_modules/@stencil/core": { - "version": "4.44.2", - "resolved": "https://registry.npmjs.org/@stencil/core/-/core-4.44.2.tgz", - "integrity": "sha512-TNaYdlyHfy8UF7glvZwaY6pM0AhgCOXuW5Kx3+AO+ITeaipCHWXi5tkZ6P/3HCL+hUySc60lPKSnZ4xdYIFb4g==", + "version": "4.45.1", + "resolved": "https://registry.npmjs.org/@stencil/core/-/core-4.45.1.tgz", + "integrity": "sha512-/b8CItmSwOzZwafxXwomJnN9X/Ic/Fxum9ka/RDHmADVf4DA4152uY7zitzLV+vk8JdXxKA7FKyLT3GKNo9R3A==", "license": "MIT", "bin": { "stencil": "bin/stencil" diff --git a/core/package.json b/core/package.json index 49095462d3e..4fe6e4cec85 100644 --- a/core/package.json +++ b/core/package.json @@ -1,6 +1,6 @@ { "name": "@ionic/core", - "version": "9.0.2", + "version": "9.0.5", "description": "Base components for Ionic", "engines": { "node": ">= 16" diff --git a/core/src/components.d.ts b/core/src/components.d.ts index 494260b38f9..ebfdb615049 100644 --- a/core/src/components.d.ts +++ b/core/src/components.d.ts @@ -1076,7 +1076,7 @@ export namespace Components { */ "mode"?: "ios" | "md"; /** - * Recalculate content dimensions. Called by overlays (e.g., popover) when sibling elements like headers or footers have finished rendering and their heights are available, ensuring accurate offset-top calculations. + * Recalculates the content dimensions and whether it should size itself to its content. Called by overlays when something they own changes, such as a header finishing its render or `--height` being updated. */ "recalculateDimensions": () => Promise; /** diff --git a/core/src/components/button/button.tsx b/core/src/components/button/button.tsx index f29f3f775d3..96cc3ff5f14 100644 --- a/core/src/components/button/button.tsx +++ b/core/src/components/button/button.tsx @@ -1,8 +1,9 @@ import type { ComponentInterface, EventEmitter } from '@stencil/core'; import { Component, Element, Event, Host, Prop, Watch, State, forceUpdate, h } from '@stencil/core'; +import type { AttributeController } from '@utils/attribute-controller'; +import { createAriaAttributeController } from '@utils/attribute-controller'; import type { AnchorInterface, ButtonInterface } from '@utils/element-interface'; -import type { Attributes } from '@utils/helpers'; -import { inheritAriaAttributes, hasShadowDom } from '@utils/helpers'; +import { hasShadowDom } from '@utils/helpers'; import { printIonWarning } from '@utils/logging'; import { createColorClasses, hostContext, openURL } from '@utils/theme'; @@ -37,7 +38,7 @@ export class Button implements ComponentInterface, AnchorInterface, ButtonInterf private inButtons = false; private formButtonEl: HTMLButtonElement | null = null; private formEl: HTMLFormElement | null = null; - private inheritedAttributes: Attributes = {}; + private ariaController?: AttributeController; @Element() el!: HTMLElement; @@ -164,27 +165,6 @@ export class Button implements ComponentInterface, AnchorInterface, ButtonInterf */ @Event() ionBlur!: EventEmitter; - /** - * This component is used within the `ion-input-password-toggle` component - * to toggle the visibility of the password input. - * These attributes need to update based on the state of the password input. - * Otherwise, the values will be stale. - * - * @param newValue - * @param _oldValue - * @param propName - */ - @Watch('aria-checked') - @Watch('aria-label') - @Watch('aria-pressed') - onAriaChanged(newValue: string, _oldValue: string, propName: string) { - this.inheritedAttributes = { - ...this.inheritedAttributes, - [propName]: newValue, - }; - forceUpdate(this); - } - /** * This is responsible for rendering a hidden native * button element inside the associated form. This allows @@ -227,7 +207,24 @@ export class Button implements ComponentInterface, AnchorInterface, ButtonInterf this.inButtons = hostContext('ion-buttons', this.el); this.inListHeader = hostContext('ion-list-header', this.el); this.inItem = hostContext('ion-item', this.el) || hostContext('ion-item-divider', this.el); - this.inheritedAttributes = inheritAriaAttributes(this.el); + + /** + * The ARIA state has to stay live, since `ion-input-password-toggle` rewrites + * `aria-label` and `aria-pressed` on its `ion-button` on every toggle. We keep + * `aria-disabled` out of the watch because the `` below renders it from the + * `disabled` prop and those writes would clobber a developer's value, and `role` out + * because a post-load write stays on the host too, which would put the same role on + * two elements in the accessibility tree. + */ + this.ariaController = createAriaAttributeController(this.el, () => forceUpdate(this), ['aria-disabled', 'role']); + } + + connectedCallback() { + this.ariaController?.init(); + } + + disconnectedCallback() { + this.ariaController?.destroy(); } private get hasIconOnly() { @@ -379,25 +376,13 @@ export class Button implements ComponentInterface, AnchorInterface, ButtonInterf }; render() { - const { - buttonType, - type, - disabled, - rel, - target, - href, - color, - expand, - hasIconOnly, - hasBadge, - strong, - inheritedAttributes, - } = this; + const { buttonType, type, disabled, rel, target, href, color, expand, hasIconOnly, hasBadge, strong } = this; const theme = getIonTheme(this); const mode = getIonMode(this); const size = this.getSize(); const shape = this.getShape(); + const inheritedAttributes = this.ariaController?.attributes ?? {}; const TagType = href === undefined ? 'button' : ('a' as any); const attrs = TagType === 'button' diff --git a/core/src/components/button/test/a11y/button.e2e.ts b/core/src/components/button/test/a11y/button.e2e.ts index 585c0b5853d..03cf866534c 100644 --- a/core/src/components/button/test/a11y/button.e2e.ts +++ b/core/src/components/button/test/a11y/button.e2e.ts @@ -148,3 +148,176 @@ configs({ directions: ['ltr'] }).forEach(({ title, screenshot, config }) => { }); }); }); + +/** + * Attribute syncing does not vary across modes or directions + */ +configs({ modes: ['md'], directions: ['ltr'] }).forEach(({ title, config }) => { + test.describe(title('button: aria attribute sync'), () => { + /** + * A sample rather than the full ARIA list, since they all go through the same + * membership check and looping every one of them only multiplies the run time. + */ + const ariaAttributes = ['aria-checked', 'aria-label', 'aria-pressed', 'aria-description']; + + for (const attr of ariaAttributes) { + test(`should sync ${attr} to the native button when it changes on the host`, async ({ page }) => { + test.info().annotations.push({ + type: 'issue', + description: 'https://github.com/ionic-team/ionic-framework/issues/30626', + }); + + await page.setContent(`Button`, config); + + const host = page.locator('ion-button'); + const nativeButton = host.locator('button'); + + await expect(nativeButton).toHaveAttribute(attr, 'initial'); + + await host.evaluate((el, attr) => el.setAttribute(attr, 'updated'), attr); + + await expect(nativeButton).toHaveAttribute(attr, 'updated'); + }); + } + + test('should not sync aria-disabled from the host', async ({ page }) => { + await page.setContent(`Button`, config); + + const host = page.locator('ion-button'); + const nativeButton = host.locator('button'); + + // The developer-provided value is still copied to the native button at load. + await expect(nativeButton).toHaveAttribute('aria-disabled', 'true'); + + // The host's `aria-disabled` belongs to the `disabled` prop from here on, so later + // writes to it must not reach the native button. We write `aria-label` in the same + // batch as a barrier, since once that lands the sync has run. + await host.evaluate((el) => { + el.setAttribute('aria-disabled', 'false'); + el.setAttribute('aria-label', 'barrier'); + }); + await expect(nativeButton).toHaveAttribute('aria-label', 'barrier'); + await expect(nativeButton).toHaveAttribute('aria-disabled', 'true'); + + // Toggling disabled makes the component write and then clear aria-disabled on the + // host. Neither write should reach the native button. + await host.evaluate((el: HTMLIonButtonElement) => { + el.disabled = true; + el.setAttribute('aria-label', 'disabled'); + }); + await expect(nativeButton).toHaveAttribute('aria-label', 'disabled'); + await expect(nativeButton).toHaveAttribute('aria-disabled', 'true'); + + await host.evaluate((el: HTMLIonButtonElement) => { + el.disabled = false; + el.setAttribute('aria-label', 'enabled'); + }); + await expect(nativeButton).toHaveAttribute('aria-label', 'enabled'); + await expect(nativeButton).toHaveAttribute('aria-disabled', 'true'); + }); + + test('should not sync role from the host', async ({ page }) => { + await page.setContent(`Button`, config); + + const host = page.locator('ion-button'); + const nativeButton = host.locator('button'); + + // The initial copy moves role onto the native button, as it always has. + await expect(nativeButton).toHaveAttribute('role', 'switch'); + + // A later write is only read, so it stays on the host. Copying it as well would put + // the same role on both elements, and two of that role in the accessibility tree. + await host.evaluate((el) => { + el.setAttribute('role', 'checkbox'); + el.setAttribute('aria-label', 'barrier'); + }); + await expect(nativeButton).toHaveAttribute('aria-label', 'barrier'); + await expect(nativeButton).toHaveAttribute('role', 'switch'); + }); + + test('should keep syncing after the button is detached and reattached', async ({ page }) => { + await page.setContent( + ` +
+ Button +
+ `, + config + ); + + const host = page.locator('ion-button'); + const nativeButton = host.locator('button'); + + await expect(nativeButton).toHaveAttribute('aria-description', 'described'); + + await host.evaluate((el) => { + const parent = el.parentElement!; + parent.removeChild(el); + parent.appendChild(el); + }); + await page.waitForChanges(); + + // The value captured at load survives the move. + await expect(nativeButton).toHaveAttribute('aria-description', 'described'); + + // Updates made after the move must still reach the native button. + await host.evaluate((el) => el.setAttribute('aria-description', 'updated')); + await expect(nativeButton).toHaveAttribute('aria-description', 'updated'); + + // So must one made while it was detached, when nothing is watching. + await host.evaluate((el) => { + const parent = el.parentElement!; + parent.removeChild(el); + el.setAttribute('aria-description', 'while detached'); + parent.appendChild(el); + }); + await expect(nativeButton).toHaveAttribute('aria-description', 'while detached'); + }); + + test('should sync updates, empty values and removals after the initial copy', async ({ page }) => { + await page.setContent(`Button`, config); + + const host = page.locator('ion-button'); + const nativeButton = host.locator('button'); + + // The initial copy moves the value from the host to the native button. + await expect(host).not.toHaveAttribute('aria-description'); + await expect(nativeButton).toHaveAttribute('aria-description', 'initial'); + + // Post-load writes stay on the host and are copied to the native button. + await host.evaluate((el) => el.setAttribute('aria-description', 'second')); + await expect(host).toHaveAttribute('aria-description', 'second'); + await expect(nativeButton).toHaveAttribute('aria-description', 'second'); + + // An empty string is a valid ARIA attribute value. + await host.evaluate((el) => el.setAttribute('aria-description', '')); + await expect(nativeButton).toHaveAttribute('aria-description', ''); + + // A removal of a post-load write does reach the native button. + await host.evaluate((el) => el.removeAttribute('aria-description')); + await expect(host).not.toHaveAttribute('aria-description'); + await expect(nativeButton).not.toHaveAttribute('aria-description'); + }); + + test('should keep a value from the initial markup when the host attribute is removed', async ({ page }) => { + await page.setContent(`Button`, config); + + const host = page.locator('ion-button'); + const nativeButton = host.locator('button'); + + await expect(nativeButton).toHaveAttribute('aria-label', 'initial'); + + // The initial copy already took the attribute off the host, so removing it there + // changes nothing and the native button keeps the copied value. Setting an empty + // value is how you clear one of these. + await host.evaluate((el) => el.removeAttribute('aria-label')); + + // Force a render and wait for it, otherwise the assertion passes on a button that + // never re-rendered at all. + await host.evaluate((el: HTMLIonButtonElement) => (el.color = 'primary')); + await expect(host).toHaveClass(/ion-color-primary/); + + await expect(nativeButton).toHaveAttribute('aria-label', 'initial'); + }); + }); +}); diff --git a/core/src/components/card/card.tsx b/core/src/components/card/card.tsx index f52b3fad8dd..683b046e41b 100644 --- a/core/src/components/card/card.tsx +++ b/core/src/components/card/card.tsx @@ -1,8 +1,8 @@ import type { ComponentInterface } from '@stencil/core'; -import { Element, Component, Host, Prop, h } from '@stencil/core'; +import { Element, Component, Host, Prop, h, forceUpdate } from '@stencil/core'; +import type { AttributeController } from '@utils/attribute-controller'; +import { createAttributeController } from '@utils/attribute-controller'; import type { AnchorInterface, ButtonInterface } from '@utils/element-interface'; -import type { Attributes } from '@utils/helpers'; -import { inheritAttributes } from '@utils/helpers'; import { createColorClasses, openURL } from '@utils/theme'; import { getIonTheme } from '../../global/ionic-global'; @@ -25,7 +25,7 @@ import type { RouterDirection } from '../router/utils/interface'; shadow: true, }) export class Card implements ComponentInterface, AnchorInterface, ButtonInterface { - private inheritedAriaAttributes: Attributes = {}; + private ariaController?: AttributeController; @Element() el!: HTMLElement; /** @@ -97,7 +97,20 @@ export class Card implements ComponentInterface, AnchorInterface, ButtonInterfac @Prop() target: string | undefined; componentWillLoad() { - this.inheritedAriaAttributes = inheritAttributes(this.el, ['aria-label']); + /** + * Only the initial copy takes the attribute off the host, so an `aria-label` written + * after load stays on the host too. That's harmless here, because unlike `ion-item` + * the card host renders no role of its own, so nothing reads the leftover copy. + */ + this.ariaController = createAttributeController(this.el, ['aria-label'], () => forceUpdate(this)); + } + + connectedCallback() { + this.ariaController?.init(); + } + + disconnectedCallback() { + this.ariaController?.destroy(); } private isClickable(): boolean { @@ -110,7 +123,8 @@ export class Card implements ComponentInterface, AnchorInterface, ButtonInterfac if (!clickable) { return []; } - const { href, routerAnimation, routerDirection, inheritedAriaAttributes } = this; + const { href, routerAnimation, routerDirection } = this; + const inheritedAriaAttributes = this.ariaController?.attributes ?? {}; const TagType = clickable ? (href === undefined ? 'button' : 'a') : ('div' as any); const attrs = TagType === 'button' diff --git a/core/src/components/card/test/a11y/card.e2e.ts b/core/src/components/card/test/a11y/card.e2e.ts index 6902037f998..7adb42ed5e7 100644 --- a/core/src/components/card/test/a11y/card.e2e.ts +++ b/core/src/components/card/test/a11y/card.e2e.ts @@ -32,3 +32,130 @@ configs({ directions: ['ltr'] }).forEach(({ title, screenshot, config }) => { }); }); }); + +/** + * Attribute syncing does not vary across modes or directions + */ +configs({ modes: ['md'], directions: ['ltr'] }).forEach(({ title, config }) => { + test.describe(title('card: aria attribute sync'), () => { + test('should sync aria-label to the native element when it changes on the host', async ({ page }) => { + test.info().annotations.push({ + type: 'issue', + description: 'https://github.com/ionic-team/ionic-framework/issues/30626', + }); + + await page.setContent(`Card`, config); + + const host = page.locator('ion-card'); + const nativeCard = host.locator('[part="native"]'); + + await expect(nativeCard).toHaveAttribute('aria-label', 'label'); + + await host.evaluate((el) => el.setAttribute('aria-label', 'updated')); + + await expect(nativeCard).toHaveAttribute('aria-label', 'updated'); + }); + + test('should keep syncing after the card is detached and reattached', async ({ page }) => { + await page.setContent( + ` +
+ Card +
+ `, + config + ); + + const host = page.locator('ion-card'); + const nativeCard = host.locator('[part="native"]'); + + await expect(nativeCard).toHaveAttribute('aria-label', 'label'); + + await host.evaluate((el) => { + const parent = el.parentElement!; + parent.removeChild(el); + parent.appendChild(el); + }); + await page.waitForChanges(); + + // The value captured at load survives the move. + await expect(nativeCard).toHaveAttribute('aria-label', 'label'); + + // Updates made after the move must still reach the native element. + await host.evaluate((el) => el.setAttribute('aria-label', 'updated')); + await expect(nativeCard).toHaveAttribute('aria-label', 'updated'); + + // So must one made while it was detached, when nothing is watching. + await host.evaluate((el) => { + const parent = el.parentElement!; + parent.removeChild(el); + el.setAttribute('aria-label', 'while detached'); + parent.appendChild(el); + }); + await expect(nativeCard).toHaveAttribute('aria-label', 'while detached'); + }); + + test('should sync updates, empty values and removals after the initial copy', async ({ page }) => { + await page.setContent(`Card`, config); + + const host = page.locator('ion-card'); + const nativeCard = host.locator('[part="native"]'); + + // The initial copy moves the value from the host to the native element. + await expect(host).not.toHaveAttribute('aria-label'); + await expect(nativeCard).toHaveAttribute('aria-label', 'initial'); + + // Post-load writes stay on the host and are copied to the native element. + await host.evaluate((el) => el.setAttribute('aria-label', 'second')); + await expect(host).toHaveAttribute('aria-label', 'second'); + await expect(nativeCard).toHaveAttribute('aria-label', 'second'); + + // An empty string is a valid ARIA attribute value. + await host.evaluate((el) => el.setAttribute('aria-label', '')); + await expect(nativeCard).toHaveAttribute('aria-label', ''); + + // A removal of a post-load write does reach the native element. + await host.evaluate((el) => el.removeAttribute('aria-label')); + await expect(host).not.toHaveAttribute('aria-label'); + await expect(nativeCard).not.toHaveAttribute('aria-label'); + }); + + test('should apply aria-label to the native element when the card becomes clickable', async ({ page }) => { + await page.setContent(`Card`, config); + + const host = page.locator('ion-card'); + await page.waitForChanges(); + + // A card that is neither a button nor a link renders no native element. + await expect(host.locator('[part="native"]')).toHaveCount(0); + + // Both `button` and `href` can be set after load, and the native element that + // appears then still needs the label copied at load. + await host.evaluate((el: HTMLIonCardElement) => (el.button = true)); + + await expect(host.locator('[part="native"]')).toHaveAttribute('aria-label', 'label'); + }); + + test('should not sync ARIA attributes other than aria-label', async ({ page }) => { + await page.setContent(`Card`, config); + + const host = page.locator('ion-card'); + const nativeCard = host.locator('[part="native"]'); + + /** + * Only `aria-label` should reach the native element. A wider watch set would put + * attributes there after load that the element never gets at load. + */ + await host.evaluate((el) => { + el.setAttribute('role', 'presentation'); + el.setAttribute('aria-describedby', 'hint'); + // Written in the same batch as a barrier, since once it lands the sync has run. + el.setAttribute('aria-label', 'updated'); + }); + + await expect(nativeCard).toHaveAttribute('aria-label', 'updated'); + await expect(nativeCard).not.toHaveAttribute('aria-describedby'); + await expect(nativeCard).not.toHaveAttribute('role'); + }); + }); +}); diff --git a/core/src/components/content/content.tsx b/core/src/components/content/content.tsx index 6fa481af5b5..4e72ecc263a 100644 --- a/core/src/components/content/content.tsx +++ b/core/src/components/content/content.tsx @@ -8,6 +8,7 @@ import { Listen, Method, Prop, + State, Watch, forceUpdate, h, @@ -15,6 +16,7 @@ import { } from '@stencil/core'; import { componentOnReady, hasLazyBuild, inheritAriaAttributes } from '@utils/helpers'; import type { Attributes } from '@utils/helpers'; +import { getOverlaySizeType } from '@utils/overlays'; import { isPlatform } from '@utils/platform'; import { isRTL } from '@utils/rtl'; import { createColorClasses, hostContext } from '@utils/theme'; @@ -81,6 +83,11 @@ export class Content implements ComponentInterface { @Element() el!: HTMLIonContentElement; + /** + * Whether the host is sized to its content. + */ + @State() sizeToContent = false; + /** * The color to use from your application's color palette. * Default options are: `"primary"`, `"secondary"`, `"tertiary"`, `"success"`, `"warning"`, `"danger"`, `"light"`, `"medium"`, and `"dark"`. @@ -152,6 +159,7 @@ export class Content implements ComponentInterface { componentWillLoad() { this.inheritedAttributes = inheritAriaAttributes(this.el); + this.sizeToContent = this.readSizeToContent(); } connectedCallback() { @@ -194,6 +202,7 @@ export class Content implements ComponentInterface { // Re-observe on reattach, since componentDidLoad only fires once. this.setupFullscreenResizeObserver(); + this.updateSizeToContent(); } componentDidLoad() { @@ -262,6 +271,17 @@ export class Content implements ComponentInterface { this.fullscreenResizeObserver.observe(this.el); } + /** + * Picks up an overlay that is no longer sized the way the last render + * assumed, re-rendering only when the answer changes. Read in a `readTask` + * because resolving the custom property forces a style recalculation. + */ + private updateSizeToContent() { + readTask(() => { + this.sizeToContent = this.readSizeToContent(); + }); + } + private destroyFullscreenResizeObserver() { if (this.fullscreenResizeObserver !== undefined) { this.fullscreenResizeObserver.disconnect(); @@ -313,6 +333,34 @@ export class Content implements ComponentInterface { return forceOverscroll === undefined ? mode === 'ios' && isPlatform('ios') : forceOverscroll; } + /** + * Reads whether to size the component to its content height. Forces a style + * recalculation, so it belongs in a read task or before the first render. + * + * This applies inside popovers and modals with a content-based `--height`, + * where the overlay does not provide the content with a definite height + * to fill. + * + * Only `--height` is consulted. Styling the wrapper directly, such as + * `ion-modal::part(content) { height: fit-content; }`, does not change + * `--height` and therefore cannot be observed. `--height` is the only + * supported way to opt into content-based sizing. + */ + private readSizeToContent() { + if (hostContext('ion-popover', this.el)) { + return true; + } + + const modal = this.el.closest('ion-modal'); + if (modal === null) { + return false; + } + + const height = getComputedStyle(modal).getPropertyValue('--height'); + + return getOverlaySizeType(height) === 'content'; + } + private resize() { /** * Only force update if the component is rendered in a browser context. @@ -323,6 +371,13 @@ export class Content implements ComponentInterface { * TODO: Remove if STENCIL-834 determines Stencil will account for this. */ if (Build.isBrowser) { + /** + * A window resize can cross a media query that changes the modal's + * `--height`. The content's own offsets are unchanged, so neither branch + * below re-renders and the class from the last render would go stale. + */ + this.updateSizeToContent(); + if (this.fullscreen) { readTask(() => this.readDimensions()); } else if (this.cTop !== 0 || this.cBottom !== 0) { @@ -333,14 +388,16 @@ export class Content implements ComponentInterface { } /** - * Recalculate content dimensions. Called by overlays (e.g., popover) when - * sibling elements like headers or footers have finished rendering and their - * heights are available, ensuring accurate offset-top calculations. + * Recalculates the content dimensions and whether it should size itself to + * its content. Called by overlays when something they own changes, such as + * a header finishing its render or `--height` being updated. + * * @internal */ @Method() async recalculateDimensions(): Promise { readTask(() => this.readDimensions()); + this.updateSizeToContent(); } private readDimensions() { @@ -542,7 +599,7 @@ export class Content implements ComponentInterface { class={createColorClasses(this.color, { [theme]: true, 'content-fullscreen': this.fullscreen, - 'content-sizing': hostContext('ion-popover', this.el), + 'content-sizing': this.sizeToContent, overscroll: forceOverscroll, [`content-${rtl}`]: true, })} diff --git a/core/src/components/datetime-button/test/overlays/datetime-button.e2e.ts b/core/src/components/datetime-button/test/overlays/datetime-button.e2e.ts index 16050dbbf1b..ab8a0c2057b 100644 --- a/core/src/components/datetime-button/test/overlays/datetime-button.e2e.ts +++ b/core/src/components/datetime-button/test/overlays/datetime-button.e2e.ts @@ -267,6 +267,24 @@ configs({ modes: ['md'], directions: ['ltr'] }).forEach(({ title, config }) => { await expect(selectedDay).toBeInViewport(); }); + test('should keep the calendar visible across repeated open and dismiss cycles', async ({ page }, testInfo) => { + testInfo.annotations.push({ + type: 'issue', + description: 'https://github.com/ionic-team/ionic-framework/issues/30933', + }); + + const calendarBody = datetime.locator('.calendar-body'); + + for (let cycle = 0; cycle < 10; cycle++) { + await openModal(page); + + await expect(calendarBody).toHaveCSS('opacity', '1'); + await expect(monthYear).toHaveText('March 2022'); + + await dismissModal(); + } + }); + test('should navigate to the previous month when reopened', async ({ page }, testInfo) => { testInfo.annotations.push({ type: 'issue', diff --git a/core/src/components/datetime/datetime.tsx b/core/src/components/datetime/datetime.tsx index 451762dae5f..7bc145d116d 100644 --- a/core/src/components/datetime/datetime.tsx +++ b/core/src/components/datetime/datetime.tsx @@ -138,16 +138,6 @@ export class Datetime implements ComponentInterface { private todayParts!: DatetimeParts; private defaultParts!: DatetimeParts; private loadTimeout: ReturnType | undefined; - /** - * Set true only by `visibleCallback`. Lets `hiddenCallback` ignore the - * synthetic "not intersecting" entry IntersectionObserver fires on - * `observe()` when the host mounts offscreen. - * - * Don't reset this in `disconnectedCallback`. Overlays disconnect and - * reconnect the host without re-creating the observers, so a reset there - * makes `hiddenCallback` miss the dismissal. - */ - private hasBeenIntersecting = false; private prevPresentation: string | null = null; @@ -1160,14 +1150,23 @@ export class Datetime implements ComponentInterface { return; } - const rect = this.el.getBoundingClientRect(); - if (rect.width === 0 || rect.height === 0) { + if (!this.hasLayoutBox()) { return; } this.markReady(); }; + /** + * Whether the datetime is on screen. A modal or popover hides its contents + * with `display: none`, which leaves the host without a layout box. + */ + private hasLayoutBox = () => { + const { width, height } = this.el.getBoundingClientRect(); + + return width > 0 && height > 0; + }; + private markReady = () => { if (this.el.classList.contains('datetime-ready')) { return; @@ -1205,12 +1204,15 @@ export class Datetime implements ComponentInterface { * areas will not have the correct values snapped into place. */ const visibleCallback = (entries: IntersectionObserverEntry[]) => { - const ev = entries[0]; + /** + * The browser can batch several observations into one callback, so + * only the last entry describes the datetime now. + */ + const ev = entries[entries.length - 1]; if (!ev.isIntersecting) { return; } - this.hasBeenIntersecting = true; this.markReady(); }; const visibleIO = new IntersectionObserver(visibleCallback, { threshold: 0.01, root: el }); @@ -1246,16 +1248,23 @@ export class Datetime implements ComponentInterface { * we did originally has been lost. */ const hiddenCallback = (entries: IntersectionObserverEntry[]) => { - const ev = entries[0]; + const ev = entries[entries.length - 1]; if (ev.isIntersecting) { return; } - // Ignore the initial "not intersecting" entry IntersectionObserver fires on observe(). - if (!this.hasBeenIntersecting) { + /** + * WebKit reports a datetime that is still on screen as not + * intersecting, and it doesn't deliver that entry to every observer on + * the same root and target, so `visibleCallback` may never hear about + * it and add `datetime-ready` back. That is what left the calendar + * blank in #30933. Checking the host instead of trusting the entry + * also covers the synthetic "not intersecting" entry `observe()` fires + * when the datetime mounts offscreen. + */ + if (this.hasLayoutBox()) { return; } - this.hasBeenIntersecting = false; this.destroyInteractionListeners(); diff --git a/core/src/components/datetime/test/basic/datetime.e2e.ts b/core/src/components/datetime/test/basic/datetime.e2e.ts index 55b5630786b..3d2e749fb43 100644 --- a/core/src/components/datetime/test/basic/datetime.e2e.ts +++ b/core/src/components/datetime/test/basic/datetime.e2e.ts @@ -579,6 +579,107 @@ configs({ modes: ['md'], directions: ['ltr'] }).forEach(({ title, config }) => { }); }); +/** + * WebKit can report a datetime that is still on screen as not intersecting, + * so the datetime must not tear down its ready state on that alone. + */ +configs({ modes: ['md'], directions: ['ltr'] }).forEach(({ title, config }) => { + test.describe(title('datetime: spurious hidden report'), () => { + test('should stay ready when the observer reports it hidden while it is on screen', async ({ page }, testInfo) => { + testInfo.annotations.push({ + type: 'issue', + description: 'https://github.com/ionic-team/ionic-framework/issues/30933', + }); + + await page.addInitScript(() => { + const OriginalIO = window.IntersectionObserver; + const datetimeObservers: { + callback: IntersectionObserverCallback; + targets: Element[]; + sawVisible: boolean; + }[] = []; + let reportedHidden = false; + + /** + * The datetime only tears down once its observers have reported it + * visible, so the test waits for that first. + */ + (window as any).datetimeObserversSawVisible = () => + datetimeObservers.length > 0 && datetimeObservers.every(({ sawVisible }) => sawVisible); + + /** + * Reports the datetime as not intersecting and then goes quiet, + * since WebKit never sends a recovery entry to undo it. + */ + (window as any).reportHiddenToDatetimeObservers = () => { + reportedHidden = true; + datetimeObservers.forEach(({ callback, targets }) => { + targets.forEach((target) => { + callback([{ isIntersecting: false, target } as IntersectionObserverEntry], null as any); + }); + }); + }; + + (window as any).IntersectionObserver = function ( + callback: IntersectionObserverCallback, + options?: IntersectionObserverInit + ) { + const root = options?.root as Element | null; + + if (root?.tagName !== 'ION-DATETIME') { + return new OriginalIO(callback, options); + } + + const record = { callback, targets: [] as Element[], sawVisible: false }; + datetimeObservers.push(record); + + const instance = new OriginalIO((entries, observer) => { + if (reportedHidden) { + return; + } + + if (entries.some((entry) => entry.isIntersecting)) { + record.sawVisible = true; + } + + callback(entries, observer); + }, options); + + const originalObserve = instance.observe.bind(instance); + instance.observe = (target: Element) => { + record.targets.push(target); + originalObserve(target); + }; + + return instance; + } as any; + }); + + await page.setContent(``, config); + + const datetime = page.locator('ion-datetime'); + const calendarBody = datetime.locator('.calendar-body'); + + await expect(datetime).toHaveClass(/datetime-ready/); + await expect(calendarBody).toHaveCSS('opacity', '1'); + await page.waitForFunction(() => (window as any).datetimeObserversSawVisible()); + + /** + * A one-shot layout fallback runs 100ms after the datetime loads and + * would add the class back. Real reports arrive long after that, so + * wait it out. + */ + await page.waitForTimeout(300); + + await page.evaluate(() => (window as any).reportHiddenToDatetimeObservers()); + await page.waitForChanges(); + + await expect(datetime).toHaveClass(/datetime-ready/); + await expect(calendarBody).toHaveCSS('opacity', '1'); + }); + }); +}); + /** * We are setting RTL on the component instead, so we don't need to test * both directions. Also, this behavior does not vary across modes. diff --git a/core/src/components/input/input.common.scss b/core/src/components/input/input.common.scss index 55f1801009b..2c67fe8ef79 100644 --- a/core/src/components/input/input.common.scss +++ b/core/src/components/input/input.common.scss @@ -79,6 +79,7 @@ flex: 1; width: 100%; + max-width: 100%; // Ensure the input fills the full height of the native wrapper. diff --git a/core/src/components/input/input.native.scss b/core/src/components/input/input.native.scss index a81c16693eb..e3b7ea613d5 100644 --- a/core/src/components/input/input.native.scss +++ b/core/src/components/input/input.native.scss @@ -209,3 +209,16 @@ margin-inline-start: $form-control-label-margin; margin-inline-end: 0; } + +// The minimum width the editable area may shrink to. Only the native themes +// carry it; the ionic theme sizes its field with its own tokens. +// -------------------------------------------------- + +.native-input { + /** + * The input keeps a minimum width so that the value stays visible + * when a long label or wide slotted content would otherwise shrink + * it to nothing. + */ + min-width: $form-control-min-width; +} diff --git a/core/src/components/input/input.tsx b/core/src/components/input/input.tsx index 316c8ca1045..7f7d5ea0581 100644 --- a/core/src/components/input/input.tsx +++ b/core/src/components/input/input.tsx @@ -13,9 +13,10 @@ import { forceUpdate, h, } from '@stencil/core'; -import type { NotchController, StartContainerController } from '@utils/forms'; +import type { ClickController, NotchController, StartContainerController } from '@utils/forms'; import { createClearButtonPressController, + createClickController, createNotchController, createStartContainerController, checkInvalidState, @@ -65,6 +66,7 @@ export class Input implements ComponentInterface { private notchSpacerEl: HTMLElement | undefined; private startContainerController?: StartContainerController; private startContainerEl: HTMLElement | undefined; + private clickController?: ClickController; private originalIonInput?: EventEmitter; @@ -421,11 +423,7 @@ export class Input implements ComponentInterface { */ @Listen('click', { capture: true }) onClickCapture(ev: Event) { - const nativeInput = this.nativeInput; - if (nativeInput && ev.target === nativeInput) { - ev.stopPropagation(); - this.el.click(); - } + this.clickController?.handleClickCapture(ev); } componentWillLoad() { @@ -466,6 +464,8 @@ export class Input implements ComponentInterface { this.startContainerController.calculateStartContainerWidth(); + this.clickController = createClickController(el, () => this.nativeInput); + // Watch for class changes to update validation state if (Build.isBrowser && typeof MutationObserver !== 'undefined') { this.validationObserver = new MutationObserver(() => { diff --git a/core/src/components/input/test/basic/input.e2e.ts b/core/src/components/input/test/basic/input.e2e.ts index 7ae4828ee91..988c0a6ea85 100644 --- a/core/src/components/input/test/basic/input.e2e.ts +++ b/core/src/components/input/test/basic/input.e2e.ts @@ -568,3 +568,81 @@ configs({ modes: ['ionic-md'], directions: ['ltr'] }).forEach(({ title, config } }); }); }); + +/** + * This behavior does not vary across directions/modes + */ +configs({ modes: ['md'], directions: ['ltr'] }).forEach(({ title, config }) => { + test.describe(title('input: slotted click'), () => { + test.beforeEach(async ({ page }) => { + await page.setContent( + ` + + + + + + + + + + `, + config + ); + }); + + test('should emit one click and focus the input when a slotted icon is clicked', async ({ page }) => { + const clickEvent = await page.spyOnEvent('click'); + + await page.locator('ion-icon[slot="start"]').click(); + + expect(clickEvent).toHaveReceivedEventTimes(1); + + const event = clickEvent.events[0]; + expect((event.target as HTMLElement).tagName.toLowerCase()).toBe('ion-icon'); + + await expect(page.locator('ion-input input.native-input')).toBeFocused(); + }); + + test('should emit one click without focusing the input when a slotted button is clicked', async ({ page }) => { + const clickEvent = await page.spyOnEvent('click'); + + await page.locator('ion-button[slot="end"]').click(); + + expect(clickEvent).toHaveReceivedEventTimes(1); + + await expect(page.locator('ion-input input.native-input')).not.toBeFocused(); + }); + + /** + * Browsers skip the label forwarding when a click lands on interactive + * content, so activating a slotted control leaves the input alone. + */ + ['ion-checkbox', 'ion-radio', 'ion-toggle'].forEach((tag) => { + test(`should activate a slotted ${tag} without focusing the input`, async ({ page }) => { + const control = page.locator(tag); + + await control.click(); + await page.waitForChanges(); + + await expect(control).toHaveAttribute('aria-checked', 'true'); + await expect(page.locator('ion-input input.native-input')).not.toBeFocused(); + }); + }); + + test('should emit one click when the input is clicked after slotted content', async ({ page }) => { + /** + * Clicking a slotted button does not produce a forwarded click for the + * input to ignore, so the following click on the input itself must + * still be emitted. + */ + await page.locator('ion-button[slot="end"]').click(); + + const clickEvent = await page.spyOnEvent('click'); + + await page.locator('ion-input input.native-input').click(); + + expect(clickEvent).toHaveReceivedEventTimes(1); + }); + }); +}); diff --git a/core/src/components/input/test/slot/input.e2e.ts b/core/src/components/input/test/slot/input.e2e.ts index 01a87d70b17..2bba6206d1b 100644 --- a/core/src/components/input/test/slot/input.e2e.ts +++ b/core/src/components/input/test/slot/input.e2e.ts @@ -322,6 +322,47 @@ configs({ modes: ['md'], directions: ['ltr'] }).forEach(({ title, screenshot, co screenshot(`input-slot-overflow-label-floating-value-${slotName}-slot`) ); }); + + /** + * The label and the input compete for whatever space the slot leaves + * behind. The input has to keep some of it so that the user can still + * see the value they are entering. + */ + test(`should not have visual regressions with a start-positioned label, a value and a wide ${slotName} slot`, async ({ + page, + }) => { + await setContent(page, 'label-placement="start" value="100"', slot); + + const container = page.locator('.container'); + await expect(container).toHaveScreenshot(screenshot(`input-slot-overflow-label-start-value-${slotName}-slot`)); + }); + + test(`should keep two characters of the value visible with a start-positioned label and a wide ${slotName} slot`, async ({ + page, + }) => { + await setContent(page, 'label-placement="start" value="100"', slot); + + const nativeInput = page.locator('ion-input input'); + + /** + * The width of two characters in the input's own font, which is what + * `$form-control-min-width` reserves for the value. + */ + const twoCharacterWidth = await nativeInput.evaluate((el) => { + const probe = document.createElement('span'); + probe.style.cssText = `position: absolute; visibility: hidden; white-space: pre; font: ${ + getComputedStyle(el).font + }`; + probe.textContent = '00'; + document.body.append(probe); + const { width } = probe.getBoundingClientRect(); + probe.remove(); + return width; + }); + const box = await nativeInput.boundingBox(); + + expect(box!.width).toBeGreaterThanOrEqual(twoCharacterWidth); + }); }); }); }); diff --git a/core/src/components/input/test/slot/input.e2e.ts-snapshots/input-slot-overflow-label-start-end-slot-md-ltr-Mobile-Chrome-linux.png b/core/src/components/input/test/slot/input.e2e.ts-snapshots/input-slot-overflow-label-start-end-slot-md-ltr-Mobile-Chrome-linux.png index 5c17032d0da..91a2cef6d8f 100644 Binary files a/core/src/components/input/test/slot/input.e2e.ts-snapshots/input-slot-overflow-label-start-end-slot-md-ltr-Mobile-Chrome-linux.png and b/core/src/components/input/test/slot/input.e2e.ts-snapshots/input-slot-overflow-label-start-end-slot-md-ltr-Mobile-Chrome-linux.png differ diff --git a/core/src/components/input/test/slot/input.e2e.ts-snapshots/input-slot-overflow-label-start-end-slot-md-ltr-Mobile-Firefox-linux.png b/core/src/components/input/test/slot/input.e2e.ts-snapshots/input-slot-overflow-label-start-end-slot-md-ltr-Mobile-Firefox-linux.png index a08604737c8..0b5bea23e9f 100644 Binary files a/core/src/components/input/test/slot/input.e2e.ts-snapshots/input-slot-overflow-label-start-end-slot-md-ltr-Mobile-Firefox-linux.png and b/core/src/components/input/test/slot/input.e2e.ts-snapshots/input-slot-overflow-label-start-end-slot-md-ltr-Mobile-Firefox-linux.png differ diff --git a/core/src/components/input/test/slot/input.e2e.ts-snapshots/input-slot-overflow-label-start-end-slot-md-ltr-Mobile-Safari-linux.png b/core/src/components/input/test/slot/input.e2e.ts-snapshots/input-slot-overflow-label-start-end-slot-md-ltr-Mobile-Safari-linux.png index 4f300336cf5..f397b3e70eb 100644 Binary files a/core/src/components/input/test/slot/input.e2e.ts-snapshots/input-slot-overflow-label-start-end-slot-md-ltr-Mobile-Safari-linux.png and b/core/src/components/input/test/slot/input.e2e.ts-snapshots/input-slot-overflow-label-start-end-slot-md-ltr-Mobile-Safari-linux.png differ diff --git a/core/src/components/input/test/slot/input.e2e.ts-snapshots/input-slot-overflow-label-start-start-slot-md-ltr-Mobile-Chrome-linux.png b/core/src/components/input/test/slot/input.e2e.ts-snapshots/input-slot-overflow-label-start-start-slot-md-ltr-Mobile-Chrome-linux.png index 940a247cee0..faa683a84dc 100644 Binary files a/core/src/components/input/test/slot/input.e2e.ts-snapshots/input-slot-overflow-label-start-start-slot-md-ltr-Mobile-Chrome-linux.png and b/core/src/components/input/test/slot/input.e2e.ts-snapshots/input-slot-overflow-label-start-start-slot-md-ltr-Mobile-Chrome-linux.png differ diff --git a/core/src/components/input/test/slot/input.e2e.ts-snapshots/input-slot-overflow-label-start-start-slot-md-ltr-Mobile-Firefox-linux.png b/core/src/components/input/test/slot/input.e2e.ts-snapshots/input-slot-overflow-label-start-start-slot-md-ltr-Mobile-Firefox-linux.png index 51f54378272..eb1aea69640 100644 Binary files a/core/src/components/input/test/slot/input.e2e.ts-snapshots/input-slot-overflow-label-start-start-slot-md-ltr-Mobile-Firefox-linux.png and b/core/src/components/input/test/slot/input.e2e.ts-snapshots/input-slot-overflow-label-start-start-slot-md-ltr-Mobile-Firefox-linux.png differ diff --git a/core/src/components/input/test/slot/input.e2e.ts-snapshots/input-slot-overflow-label-start-start-slot-md-ltr-Mobile-Safari-linux.png b/core/src/components/input/test/slot/input.e2e.ts-snapshots/input-slot-overflow-label-start-start-slot-md-ltr-Mobile-Safari-linux.png index b025d8c9f11..57b333e8a89 100644 Binary files a/core/src/components/input/test/slot/input.e2e.ts-snapshots/input-slot-overflow-label-start-start-slot-md-ltr-Mobile-Safari-linux.png and b/core/src/components/input/test/slot/input.e2e.ts-snapshots/input-slot-overflow-label-start-start-slot-md-ltr-Mobile-Safari-linux.png differ diff --git a/core/src/components/input/test/slot/input.e2e.ts-snapshots/input-slot-overflow-label-start-value-end-slot-md-ltr-Mobile-Chrome-linux.png b/core/src/components/input/test/slot/input.e2e.ts-snapshots/input-slot-overflow-label-start-value-end-slot-md-ltr-Mobile-Chrome-linux.png new file mode 100644 index 00000000000..64dc04bb198 Binary files /dev/null and b/core/src/components/input/test/slot/input.e2e.ts-snapshots/input-slot-overflow-label-start-value-end-slot-md-ltr-Mobile-Chrome-linux.png differ diff --git a/core/src/components/input/test/slot/input.e2e.ts-snapshots/input-slot-overflow-label-start-value-end-slot-md-ltr-Mobile-Firefox-linux.png b/core/src/components/input/test/slot/input.e2e.ts-snapshots/input-slot-overflow-label-start-value-end-slot-md-ltr-Mobile-Firefox-linux.png new file mode 100644 index 00000000000..2868ea41277 Binary files /dev/null and b/core/src/components/input/test/slot/input.e2e.ts-snapshots/input-slot-overflow-label-start-value-end-slot-md-ltr-Mobile-Firefox-linux.png differ diff --git a/core/src/components/input/test/slot/input.e2e.ts-snapshots/input-slot-overflow-label-start-value-end-slot-md-ltr-Mobile-Safari-linux.png b/core/src/components/input/test/slot/input.e2e.ts-snapshots/input-slot-overflow-label-start-value-end-slot-md-ltr-Mobile-Safari-linux.png new file mode 100644 index 00000000000..5c63e43eba4 Binary files /dev/null and b/core/src/components/input/test/slot/input.e2e.ts-snapshots/input-slot-overflow-label-start-value-end-slot-md-ltr-Mobile-Safari-linux.png differ diff --git a/core/src/components/input/test/slot/input.e2e.ts-snapshots/input-slot-overflow-label-start-value-start-slot-md-ltr-Mobile-Chrome-linux.png b/core/src/components/input/test/slot/input.e2e.ts-snapshots/input-slot-overflow-label-start-value-start-slot-md-ltr-Mobile-Chrome-linux.png new file mode 100644 index 00000000000..dba097a6145 Binary files /dev/null and b/core/src/components/input/test/slot/input.e2e.ts-snapshots/input-slot-overflow-label-start-value-start-slot-md-ltr-Mobile-Chrome-linux.png differ diff --git a/core/src/components/input/test/slot/input.e2e.ts-snapshots/input-slot-overflow-label-start-value-start-slot-md-ltr-Mobile-Firefox-linux.png b/core/src/components/input/test/slot/input.e2e.ts-snapshots/input-slot-overflow-label-start-value-start-slot-md-ltr-Mobile-Firefox-linux.png new file mode 100644 index 00000000000..af7d68dc5b2 Binary files /dev/null and b/core/src/components/input/test/slot/input.e2e.ts-snapshots/input-slot-overflow-label-start-value-start-slot-md-ltr-Mobile-Firefox-linux.png differ diff --git a/core/src/components/input/test/slot/input.e2e.ts-snapshots/input-slot-overflow-label-start-value-start-slot-md-ltr-Mobile-Safari-linux.png b/core/src/components/input/test/slot/input.e2e.ts-snapshots/input-slot-overflow-label-start-value-start-slot-md-ltr-Mobile-Safari-linux.png new file mode 100644 index 00000000000..7cb78643e1f Binary files /dev/null and b/core/src/components/input/test/slot/input.e2e.ts-snapshots/input-slot-overflow-label-start-value-start-slot-md-ltr-Mobile-Safari-linux.png differ diff --git a/core/src/components/item/item.tsx b/core/src/components/item/item.tsx index 1564043a8c7..82882f17eb0 100644 --- a/core/src/components/item/item.tsx +++ b/core/src/components/item/item.tsx @@ -1,8 +1,9 @@ import type { ComponentInterface } from '@stencil/core'; import { Build, Component, Element, Host, Listen, Prop, State, Watch, forceUpdate, h } from '@stencil/core'; +import type { AttributeController } from '@utils/attribute-controller'; +import { createAttributeController } from '@utils/attribute-controller'; import type { AnchorInterface, ButtonInterface } from '@utils/element-interface'; -import type { Attributes } from '@utils/helpers'; -import { inheritAttributes, raf } from '@utils/helpers'; +import { raf } from '@utils/helpers'; import { createColorClasses, hostContext, openURL } from '@utils/theme'; import { chevronForward } from 'ionicons/icons'; @@ -38,9 +39,9 @@ const INDICATOR_CONTROL_SELECTOR = 'ion-checkbox, ion-radio, ion-toggle'; export class Item implements ComponentInterface, AnchorInterface, ButtonInterface { private labelColorStyles = {}; private itemStyles = new Map(); - private inheritedAriaAttributes: Attributes = {}; private indicatorControlObserver?: MutationObserver; private didLoad = false; + private ariaController?: AttributeController; @Element() el!: HTMLIonItemElement; @@ -183,10 +184,18 @@ export class Item implements ComponentInterface, AnchorInterface, ButtonInterfac this.watchForIndicatorControls(); this.updateInteractivityOnSlotChange(); } + + this.ariaController?.init(); } componentWillLoad() { - this.inheritedAriaAttributes = inheritAttributes(this.el, ['aria-label']); + /** + * Only the initial copy takes the attribute off the host, so an `aria-label` written + * after load names both the native element and the Host, which is a `listitem` when + * the item is in an `ion-list`. The two names always agree, so a screen reader just + * reads it twice. + */ + this.ariaController = createAttributeController(this.el, ['aria-label'], () => forceUpdate(this)); } componentDidLoad() { @@ -206,6 +215,8 @@ export class Item implements ComponentInterface, AnchorInterface, ButtonInterfac this.indicatorControlObserver.disconnect(); this.indicatorControlObserver = undefined; } + + this.ariaController?.destroy(); } private totalNestedInputs() { @@ -393,9 +404,9 @@ export class Item implements ComponentInterface, AnchorInterface, ButtonInterfac target, routerAnimation, routerDirection, - inheritedAriaAttributes, multipleInputs, } = this; + const inheritedAriaAttributes = this.ariaController?.attributes ?? {}; const childStyles = {} as StyleEventDetail; const theme = getIonTheme(this); const clickable = this.isClickable(); diff --git a/core/src/components/item/test/a11y/item.e2e.ts b/core/src/components/item/test/a11y/item.e2e.ts index 20536beb71a..084e4d3b65e 100644 --- a/core/src/components/item/test/a11y/item.e2e.ts +++ b/core/src/components/item/test/a11y/item.e2e.ts @@ -153,3 +153,115 @@ configs({ directions: ['ltr'] }).forEach(({ config, screenshot, title }) => { }); }); }); + +/** + * Attribute syncing does not vary across modes or directions + */ +configs({ modes: ['md'], directions: ['ltr'] }).forEach(({ title, config }) => { + test.describe(title('item: aria attribute sync'), () => { + test('should sync aria-label to the native element when it changes on the host', async ({ page }) => { + test.info().annotations.push({ + type: 'issue', + description: 'https://github.com/ionic-team/ionic-framework/issues/30626', + }); + + await page.setContent(`Item`, config); + + const host = page.locator('ion-item'); + const nativeItem = host.locator('[part="native"]'); + + await expect(nativeItem).toHaveAttribute('aria-label', 'label'); + + await host.evaluate((el) => el.setAttribute('aria-label', 'updated')); + + await expect(nativeItem).toHaveAttribute('aria-label', 'updated'); + }); + + test('should keep syncing after the item is detached and reattached', async ({ page }) => { + await page.setContent( + ` +
+ Item +
+ `, + config + ); + + const host = page.locator('ion-item'); + const nativeItem = host.locator('[part="native"]'); + + await expect(nativeItem).toHaveAttribute('aria-label', 'label'); + + await host.evaluate((el) => { + const parent = el.parentElement!; + parent.removeChild(el); + parent.appendChild(el); + }); + await page.waitForChanges(); + + // The value captured at load survives the move. + await expect(nativeItem).toHaveAttribute('aria-label', 'label'); + + // Updates made after the move must still reach the native element. + await host.evaluate((el) => el.setAttribute('aria-label', 'updated')); + await expect(nativeItem).toHaveAttribute('aria-label', 'updated'); + + // So must one made while it was detached, when nothing is watching. + await host.evaluate((el) => { + const parent = el.parentElement!; + parent.removeChild(el); + el.setAttribute('aria-label', 'while detached'); + parent.appendChild(el); + }); + await expect(nativeItem).toHaveAttribute('aria-label', 'while detached'); + }); + + test('should sync updates, empty values and removals after the initial copy', async ({ page }) => { + await page.setContent(`Item`, config); + + const host = page.locator('ion-item'); + const nativeItem = host.locator('[part="native"]'); + + // The initial copy moves the value from the host to the native element. + await expect(host).not.toHaveAttribute('aria-label'); + await expect(nativeItem).toHaveAttribute('aria-label', 'initial'); + + // Post-load writes stay on the host and are copied to the native element. + await host.evaluate((el) => el.setAttribute('aria-label', 'second')); + await expect(host).toHaveAttribute('aria-label', 'second'); + await expect(nativeItem).toHaveAttribute('aria-label', 'second'); + + // An empty string is a valid ARIA attribute value. + await host.evaluate((el) => el.setAttribute('aria-label', '')); + await expect(nativeItem).toHaveAttribute('aria-label', ''); + + // A removal of a post-load write does reach the native element. + await host.evaluate((el) => el.removeAttribute('aria-label')); + await expect(host).not.toHaveAttribute('aria-label'); + await expect(nativeItem).not.toHaveAttribute('aria-label'); + }); + + test('should not sync ARIA attributes other than aria-label', async ({ page }) => { + await page.setContent(`Item`, config); + + const host = page.locator('ion-item'); + const nativeItem = host.locator('[part="native"]'); + + /** + * Only `aria-label` should reach the native element. An `ion-item` in a list renders + * `role="listitem"` on its own Host, so watching `role` would copy that onto the + * native button. + */ + await host.evaluate((el) => { + el.setAttribute('role', 'presentation'); + el.setAttribute('aria-describedby', 'hint'); + // Written in the same batch as a barrier, since once it lands the sync has run. + el.setAttribute('aria-label', 'updated'); + }); + + await expect(nativeItem).toHaveAttribute('aria-label', 'updated'); + await expect(nativeItem).not.toHaveAttribute('aria-describedby'); + await expect(nativeItem).not.toHaveAttribute('role'); + }); + }); +}); diff --git a/core/src/components/item/test/slotted-inputs/item.e2e.ts-snapshots/item-slotted-inputs-ios-ltr-Mobile-Chrome-linux.png b/core/src/components/item/test/slotted-inputs/item.e2e.ts-snapshots/item-slotted-inputs-ios-ltr-Mobile-Chrome-linux.png index 2f59aebd92f..5b7d06b3c85 100644 Binary files a/core/src/components/item/test/slotted-inputs/item.e2e.ts-snapshots/item-slotted-inputs-ios-ltr-Mobile-Chrome-linux.png and b/core/src/components/item/test/slotted-inputs/item.e2e.ts-snapshots/item-slotted-inputs-ios-ltr-Mobile-Chrome-linux.png differ diff --git a/core/src/components/item/test/slotted-inputs/item.e2e.ts-snapshots/item-slotted-inputs-ios-ltr-Mobile-Firefox-linux.png b/core/src/components/item/test/slotted-inputs/item.e2e.ts-snapshots/item-slotted-inputs-ios-ltr-Mobile-Firefox-linux.png index ba95f29938a..9873fe7b940 100644 Binary files a/core/src/components/item/test/slotted-inputs/item.e2e.ts-snapshots/item-slotted-inputs-ios-ltr-Mobile-Firefox-linux.png and b/core/src/components/item/test/slotted-inputs/item.e2e.ts-snapshots/item-slotted-inputs-ios-ltr-Mobile-Firefox-linux.png differ diff --git a/core/src/components/item/test/slotted-inputs/item.e2e.ts-snapshots/item-slotted-inputs-ios-ltr-Mobile-Safari-linux.png b/core/src/components/item/test/slotted-inputs/item.e2e.ts-snapshots/item-slotted-inputs-ios-ltr-Mobile-Safari-linux.png index f18894350bf..dd5eed9b793 100644 Binary files a/core/src/components/item/test/slotted-inputs/item.e2e.ts-snapshots/item-slotted-inputs-ios-ltr-Mobile-Safari-linux.png and b/core/src/components/item/test/slotted-inputs/item.e2e.ts-snapshots/item-slotted-inputs-ios-ltr-Mobile-Safari-linux.png differ diff --git a/core/src/components/item/test/slotted-inputs/item.e2e.ts-snapshots/item-slotted-inputs-ios-rtl-Mobile-Chrome-linux.png b/core/src/components/item/test/slotted-inputs/item.e2e.ts-snapshots/item-slotted-inputs-ios-rtl-Mobile-Chrome-linux.png index 027bc983134..19c9b3722a9 100644 Binary files a/core/src/components/item/test/slotted-inputs/item.e2e.ts-snapshots/item-slotted-inputs-ios-rtl-Mobile-Chrome-linux.png and b/core/src/components/item/test/slotted-inputs/item.e2e.ts-snapshots/item-slotted-inputs-ios-rtl-Mobile-Chrome-linux.png differ diff --git a/core/src/components/item/test/slotted-inputs/item.e2e.ts-snapshots/item-slotted-inputs-ios-rtl-Mobile-Firefox-linux.png b/core/src/components/item/test/slotted-inputs/item.e2e.ts-snapshots/item-slotted-inputs-ios-rtl-Mobile-Firefox-linux.png index c1c1f34640f..2e8c6f66897 100644 Binary files a/core/src/components/item/test/slotted-inputs/item.e2e.ts-snapshots/item-slotted-inputs-ios-rtl-Mobile-Firefox-linux.png and b/core/src/components/item/test/slotted-inputs/item.e2e.ts-snapshots/item-slotted-inputs-ios-rtl-Mobile-Firefox-linux.png differ diff --git a/core/src/components/item/test/slotted-inputs/item.e2e.ts-snapshots/item-slotted-inputs-ios-rtl-Mobile-Safari-linux.png b/core/src/components/item/test/slotted-inputs/item.e2e.ts-snapshots/item-slotted-inputs-ios-rtl-Mobile-Safari-linux.png index c894a066f40..8e308b6db2e 100644 Binary files a/core/src/components/item/test/slotted-inputs/item.e2e.ts-snapshots/item-slotted-inputs-ios-rtl-Mobile-Safari-linux.png and b/core/src/components/item/test/slotted-inputs/item.e2e.ts-snapshots/item-slotted-inputs-ios-rtl-Mobile-Safari-linux.png differ diff --git a/core/src/components/item/test/slotted-inputs/item.e2e.ts-snapshots/item-slotted-inputs-md-ltr-Mobile-Chrome-linux.png b/core/src/components/item/test/slotted-inputs/item.e2e.ts-snapshots/item-slotted-inputs-md-ltr-Mobile-Chrome-linux.png index 198338daba3..d985c1a06d0 100644 Binary files a/core/src/components/item/test/slotted-inputs/item.e2e.ts-snapshots/item-slotted-inputs-md-ltr-Mobile-Chrome-linux.png and b/core/src/components/item/test/slotted-inputs/item.e2e.ts-snapshots/item-slotted-inputs-md-ltr-Mobile-Chrome-linux.png differ diff --git a/core/src/components/item/test/slotted-inputs/item.e2e.ts-snapshots/item-slotted-inputs-md-ltr-Mobile-Firefox-linux.png b/core/src/components/item/test/slotted-inputs/item.e2e.ts-snapshots/item-slotted-inputs-md-ltr-Mobile-Firefox-linux.png index 0ef707e079a..e11eb2f9415 100644 Binary files a/core/src/components/item/test/slotted-inputs/item.e2e.ts-snapshots/item-slotted-inputs-md-ltr-Mobile-Firefox-linux.png and b/core/src/components/item/test/slotted-inputs/item.e2e.ts-snapshots/item-slotted-inputs-md-ltr-Mobile-Firefox-linux.png differ diff --git a/core/src/components/item/test/slotted-inputs/item.e2e.ts-snapshots/item-slotted-inputs-md-ltr-Mobile-Safari-linux.png b/core/src/components/item/test/slotted-inputs/item.e2e.ts-snapshots/item-slotted-inputs-md-ltr-Mobile-Safari-linux.png index 130f37b511b..622ced1c97c 100644 Binary files a/core/src/components/item/test/slotted-inputs/item.e2e.ts-snapshots/item-slotted-inputs-md-ltr-Mobile-Safari-linux.png and b/core/src/components/item/test/slotted-inputs/item.e2e.ts-snapshots/item-slotted-inputs-md-ltr-Mobile-Safari-linux.png differ diff --git a/core/src/components/item/test/slotted-inputs/item.e2e.ts-snapshots/item-slotted-inputs-md-rtl-Mobile-Chrome-linux.png b/core/src/components/item/test/slotted-inputs/item.e2e.ts-snapshots/item-slotted-inputs-md-rtl-Mobile-Chrome-linux.png index 0d8b6331954..d7bd849bf6c 100644 Binary files a/core/src/components/item/test/slotted-inputs/item.e2e.ts-snapshots/item-slotted-inputs-md-rtl-Mobile-Chrome-linux.png and b/core/src/components/item/test/slotted-inputs/item.e2e.ts-snapshots/item-slotted-inputs-md-rtl-Mobile-Chrome-linux.png differ diff --git a/core/src/components/item/test/slotted-inputs/item.e2e.ts-snapshots/item-slotted-inputs-md-rtl-Mobile-Firefox-linux.png b/core/src/components/item/test/slotted-inputs/item.e2e.ts-snapshots/item-slotted-inputs-md-rtl-Mobile-Firefox-linux.png index 0903054ec7b..2abb49d033b 100644 Binary files a/core/src/components/item/test/slotted-inputs/item.e2e.ts-snapshots/item-slotted-inputs-md-rtl-Mobile-Firefox-linux.png and b/core/src/components/item/test/slotted-inputs/item.e2e.ts-snapshots/item-slotted-inputs-md-rtl-Mobile-Firefox-linux.png differ diff --git a/core/src/components/item/test/slotted-inputs/item.e2e.ts-snapshots/item-slotted-inputs-md-rtl-Mobile-Safari-linux.png b/core/src/components/item/test/slotted-inputs/item.e2e.ts-snapshots/item-slotted-inputs-md-rtl-Mobile-Safari-linux.png index 95f7000e1be..d68bcc43887 100644 Binary files a/core/src/components/item/test/slotted-inputs/item.e2e.ts-snapshots/item-slotted-inputs-md-rtl-Mobile-Safari-linux.png and b/core/src/components/item/test/slotted-inputs/item.e2e.ts-snapshots/item-slotted-inputs-md-rtl-Mobile-Safari-linux.png differ diff --git a/core/src/components/item/test/slotted-inputs/item.e2e.ts-snapshots/item-slotted-inputs-select-ios-ltr-Mobile-Chrome-linux.png b/core/src/components/item/test/slotted-inputs/item.e2e.ts-snapshots/item-slotted-inputs-select-ios-ltr-Mobile-Chrome-linux.png index 4fbca6ec1b4..35ed98c1d0a 100644 Binary files a/core/src/components/item/test/slotted-inputs/item.e2e.ts-snapshots/item-slotted-inputs-select-ios-ltr-Mobile-Chrome-linux.png and b/core/src/components/item/test/slotted-inputs/item.e2e.ts-snapshots/item-slotted-inputs-select-ios-ltr-Mobile-Chrome-linux.png differ diff --git a/core/src/components/item/test/slotted-inputs/item.e2e.ts-snapshots/item-slotted-inputs-select-ios-ltr-Mobile-Firefox-linux.png b/core/src/components/item/test/slotted-inputs/item.e2e.ts-snapshots/item-slotted-inputs-select-ios-ltr-Mobile-Firefox-linux.png index 69dc9c6fab3..43d06761556 100644 Binary files a/core/src/components/item/test/slotted-inputs/item.e2e.ts-snapshots/item-slotted-inputs-select-ios-ltr-Mobile-Firefox-linux.png and b/core/src/components/item/test/slotted-inputs/item.e2e.ts-snapshots/item-slotted-inputs-select-ios-ltr-Mobile-Firefox-linux.png differ diff --git a/core/src/components/item/test/slotted-inputs/item.e2e.ts-snapshots/item-slotted-inputs-select-ios-ltr-Mobile-Safari-linux.png b/core/src/components/item/test/slotted-inputs/item.e2e.ts-snapshots/item-slotted-inputs-select-ios-ltr-Mobile-Safari-linux.png index 42ba5871969..d754ce166ce 100644 Binary files a/core/src/components/item/test/slotted-inputs/item.e2e.ts-snapshots/item-slotted-inputs-select-ios-ltr-Mobile-Safari-linux.png and b/core/src/components/item/test/slotted-inputs/item.e2e.ts-snapshots/item-slotted-inputs-select-ios-ltr-Mobile-Safari-linux.png differ diff --git a/core/src/components/item/test/slotted-inputs/item.e2e.ts-snapshots/item-slotted-inputs-select-md-ltr-Mobile-Chrome-linux.png b/core/src/components/item/test/slotted-inputs/item.e2e.ts-snapshots/item-slotted-inputs-select-md-ltr-Mobile-Chrome-linux.png index fc4404242a4..3b3a5540e6e 100644 Binary files a/core/src/components/item/test/slotted-inputs/item.e2e.ts-snapshots/item-slotted-inputs-select-md-ltr-Mobile-Chrome-linux.png and b/core/src/components/item/test/slotted-inputs/item.e2e.ts-snapshots/item-slotted-inputs-select-md-ltr-Mobile-Chrome-linux.png differ diff --git a/core/src/components/item/test/slotted-inputs/item.e2e.ts-snapshots/item-slotted-inputs-select-md-ltr-Mobile-Firefox-linux.png b/core/src/components/item/test/slotted-inputs/item.e2e.ts-snapshots/item-slotted-inputs-select-md-ltr-Mobile-Firefox-linux.png index 081d6371781..90d1142361b 100644 Binary files a/core/src/components/item/test/slotted-inputs/item.e2e.ts-snapshots/item-slotted-inputs-select-md-ltr-Mobile-Firefox-linux.png and b/core/src/components/item/test/slotted-inputs/item.e2e.ts-snapshots/item-slotted-inputs-select-md-ltr-Mobile-Firefox-linux.png differ diff --git a/core/src/components/item/test/slotted-inputs/item.e2e.ts-snapshots/item-slotted-inputs-select-md-ltr-Mobile-Safari-linux.png b/core/src/components/item/test/slotted-inputs/item.e2e.ts-snapshots/item-slotted-inputs-select-md-ltr-Mobile-Safari-linux.png index 914568c6e26..a50b10d5bac 100644 Binary files a/core/src/components/item/test/slotted-inputs/item.e2e.ts-snapshots/item-slotted-inputs-select-md-ltr-Mobile-Safari-linux.png and b/core/src/components/item/test/slotted-inputs/item.e2e.ts-snapshots/item-slotted-inputs-select-md-ltr-Mobile-Safari-linux.png differ diff --git a/core/src/components/modal/modal.common.scss b/core/src/components/modal/modal.common.scss index dfe405a2ba6..b4aa8f7ba53 100644 --- a/core/src/components/modal/modal.common.scss +++ b/core/src/components/modal/modal.common.scss @@ -27,7 +27,12 @@ --max-width: auto; --height: 100%; --min-height: auto; - --max-height: auto; + /** + * Clamps a content-sized `--height` (auto, fit-content, ...) to the + * overlay, giving the wrapper's flex children something to shrink + * toward so `ion-content` scrolls instead of overflowing. + */ + --max-height: 100%; --overflow: hidden; --border-width: 0; --border-style: none; @@ -82,8 +87,16 @@ ion-backdrop { /** * The wrapper receives programmatic focus for screen readers but should not * show a visible focus ring, which is meant only for keyboard navigation. + * + * A flex layout is required for the wrapper to size itself to its content + * when the modal is content-sized (`--height` is auto, fit-content, ...). + * This makes it so that the content can scroll when it overflows the wrapper. */ .modal-wrapper { + display: flex; + + flex-direction: column; + outline: none; } diff --git a/core/src/components/modal/modal.tsx b/core/src/components/modal/modal.tsx index cf3628dc278..8f13b0665d7 100644 --- a/core/src/components/modal/modal.tsx +++ b/core/src/components/modal/modal.tsx @@ -54,10 +54,10 @@ import { clearSafeAreaOverrides, getRootSafeAreaTop, onRootSafeAreaTopChange, - hasCustomModalDimensions, + getModalCoveredAxes, type ModalSafeAreaContext, } from './safe-area-utils'; -import { setCardStatusBarDark, setCardStatusBarDefault } from './utils'; +import { onModalHeightChange, setCardStatusBarDark, setCardStatusBarDefault } from './utils'; // TODO(FW-2832): types @@ -117,6 +117,7 @@ export class Modal implements ComponentInterface, OverlayInterface { private viewTransitionAnimation?: Animation; private resizeTimeout?: any; private unsubscribeRootSafeAreaTop?: () => void; + private unsubscribeHeightChange?: () => void; // True from the first safe-area write in `present()` until the enter // animation settles. A position-based read in that window is not the rest position. private isPresenting = false; @@ -1518,7 +1519,7 @@ export class Modal implements ComponentInterface, OverlayInterface { /** * Creates the context object for safe-area utilities. * - * `hasCustomDimensions` is only set by `setInitialSafeAreaOverrides()` + * `coveredAxes` is only set by `setInitialSafeAreaOverrides()` * because it is only read by `getInitialSafeAreaConfig()`. Other callers * (resize handler, post-animation update, fullscreen-padding apply) would * pay a `getComputedStyle()` cost for a value they never consult. @@ -1533,6 +1534,28 @@ export class Modal implements ComponentInterface, OverlayInterface { }; } + /** + * Keeps the content's sizing in sync with `--height`. The content reads the + * property to determine whether it should size itself to its content, and + * changes to `--height` on an ancestor or the root can change that behavior + * without changing the modal itself. + */ + private watchHeightForContent(): void { + /** + * A sheet's height comes from its breakpoints, so its content never sizes + * itself to `--height`. Watching it would cause the drag to recalculate on + * every frame. + */ + if (this.isSheetModal) { + return; + } + + this.unsubscribeHeightChange?.(); + this.unsubscribeHeightChange = onModalHeightChange(this.el, () => { + this.el.querySelectorAll('ion-content').forEach((contentEl) => contentEl.recalculateDimensions()); + }); + } + /** * Sets initial safe-area overrides before modal animation. * Called in present() before animation starts. @@ -1546,11 +1569,13 @@ export class Modal implements ComponentInterface, OverlayInterface { private setInitialSafeAreaOverrides(): void { const context: ModalSafeAreaContext = { ...this.getSafeAreaContext(), - hasCustomDimensions: hasCustomModalDimensions(this.el), + coveredAxes: getModalCoveredAxes(this.el), }; const safeAreaConfig = getInitialSafeAreaConfig(context); applySafeAreaOverrides(this.el, safeAreaConfig); + this.watchHeightForContent(); + // Set the internal offset property with the resolved root safe-area-top value if (context.isSheetModal) { this.updateSheetOffsetTop(); @@ -1677,6 +1702,9 @@ export class Modal implements ComponentInterface, OverlayInterface { this.unsubscribeRootSafeAreaTop?.(); this.unsubscribeRootSafeAreaTop = undefined; + this.unsubscribeHeightChange?.(); + this.unsubscribeHeightChange = undefined; + // Remove internal sheet offset property this.el.style.removeProperty('--ion-modal-offset-top'); diff --git a/core/src/components/modal/safe-area-utils.spec.ts b/core/src/components/modal/safe-area-utils.spec.ts new file mode 100644 index 00000000000..af69bfcc7fd --- /dev/null +++ b/core/src/components/modal/safe-area-utils.spec.ts @@ -0,0 +1,130 @@ +import { getModalCoveredAxes } from './safe-area-utils'; + +/** + * Tests `getModalCoveredAxes()` across fullscreen, fixed-size, and + * content-sized modals. The helper uses computed CSS sizes when both + * dimensions are fullscreen and measures the rendered wrapper otherwise, + * so the tests mock both sources of size information as needed. + */ +describe('modal: getModalCoveredAxes', () => { + const VIEWPORT_WIDTH = window.innerWidth; + const VIEWPORT_HEIGHT = window.innerHeight; + + let host: HTMLElement; + let wrapper: HTMLElement; + let hiddenDuringMeasurement: boolean; + let originalGetComputedStyle: PropertyDescriptor | undefined; + let sizes: Record; + + const setSize = (width: string, height: string) => { + sizes = { '--width': width, '--height': height }; + }; + + const setWrapperBox = (width: number, height: number) => { + wrapper.getBoundingClientRect = () => { + hiddenDuringMeasurement = host.classList.contains('overlay-hidden'); + return { width, height } as DOMRect; + }; + }; + + beforeEach(() => { + host = document.createElement('ion-modal'); + host.classList.add('overlay-hidden'); + document.body.appendChild(host); + + wrapper = document.createElement('div'); + wrapper.classList.add('modal-wrapper'); + host.attachShadow({ mode: 'open' }).appendChild(wrapper); + + hiddenDuringMeasurement = true; + setWrapperBox(0, 0); + + /** + * Replace the mocked `getComputedStyle` getter so tests can control + * the modal's `--width` and `--height` values. + */ + sizes = {}; + originalGetComputedStyle = Object.getOwnPropertyDescriptor(globalThis, 'getComputedStyle'); + Object.defineProperty(globalThis, 'getComputedStyle', { + value: () => ({ getPropertyValue: (property: string) => sizes[property] ?? '' }), + configurable: true, + writable: true, + }); + }); + + afterEach(() => { + if (originalGetComputedStyle) { + Object.defineProperty(globalThis, 'getComputedStyle', originalGetComputedStyle); + } + host.remove(); + }); + + it('should cover both axes when the sizes span the viewport', () => { + setSize('100%', '100%'); + + expect(getModalCoveredAxes(host)).toEqual({ vertical: true, horizontal: true }); + }); + + it('should cover neither axis for a dialog that stays clear of the viewport edges', () => { + setSize('300px', '200px'); + setWrapperBox(300, 200); + + expect(getModalCoveredAxes(host)).toEqual({ vertical: false, horizontal: false }); + }); + + it('should cover only the horizontal axis for a full-width dialog that fits its content', () => { + setSize('100%', 'fit-content'); + setWrapperBox(VIEWPORT_WIDTH, 244); + + expect(getModalCoveredAxes(host)).toEqual({ vertical: false, horizontal: true }); + }); + + it('should cover only the vertical axis for a narrow modal that fills the viewport', () => { + setSize('300px', 'fit-content'); + setWrapperBox(300, VIEWPORT_HEIGHT); + + expect(getModalCoveredAxes(host)).toEqual({ vertical: true, horizontal: false }); + }); + + it('should cover an axis whose definite size reaches the viewport', () => { + setSize('300px', `${VIEWPORT_HEIGHT}px`); + setWrapperBox(300, VIEWPORT_HEIGHT); + + expect(getModalCoveredAxes(host)).toEqual({ vertical: true, horizontal: false }); + }); + + it('should allow a few pixels of tolerance when comparing to the viewport', () => { + setSize('300px', 'fit-content'); + setWrapperBox(300, VIEWPORT_HEIGHT - 4); + + expect(getModalCoveredAxes(host)).toEqual({ vertical: true, horizontal: false }); + }); + + it('should temporarily show a hidden modal while measuring and hide it again', () => { + setSize('300px', 'fit-content'); + setWrapperBox(300, VIEWPORT_HEIGHT); + + getModalCoveredAxes(host); + + expect(hiddenDuringMeasurement).toBe(false); + expect(host.classList.contains('overlay-hidden')).toBe(true); + }); + + it('should not change visibility when measuring an already visible modal', () => { + host.classList.remove('overlay-hidden'); + setSize('300px', 'fit-content'); + setWrapperBox(300, 244); + + getModalCoveredAxes(host); + + expect(host.classList.contains('overlay-hidden')).toBe(false); + }); + + it('should not measure when both sizes span the viewport', () => { + setSize('100%', '100vh'); + setWrapperBox(0, 0); + + expect(getModalCoveredAxes(host)).toEqual({ vertical: true, horizontal: true }); + expect(hiddenDuringMeasurement).toBe(true); + }); +}); diff --git a/core/src/components/modal/safe-area-utils.ts b/core/src/components/modal/safe-area-utils.ts index b06b2315b63..5d0f7d9150d 100644 --- a/core/src/components/modal/safe-area-utils.ts +++ b/core/src/components/modal/safe-area-utils.ts @@ -1,5 +1,6 @@ import { win } from '@utils/browser'; -import { raf } from '@utils/helpers'; +import { onCustomPropertyChange, raf } from '@utils/helpers'; +import { getOverlaySizeType } from '@utils/overlays'; type SafeAreaValue = '0px' | 'inherit'; @@ -14,6 +15,17 @@ export interface SafeAreaConfig { right: SafeAreaValue; } +/** + * Indicates whether the modal spans the viewport on each axis. + * + * `vertical` means the modal reaches both the top and bottom edges. + * `horizontal` means the modal reaches both the left and right edges. + */ +export interface ModalCoveredAxes { + vertical: boolean; + horizontal: boolean; +} + /** * Context information about the modal used to determine safe-area behavior. */ @@ -23,50 +35,23 @@ export interface ModalSafeAreaContext { presentingElement?: HTMLElement; breakpoints?: number[]; currentBreakpoint?: number; + /** - * Only consulted by `getInitialSafeAreaConfig()`. Callers that only use the - * context for non-initial paths can omit this. See `hasCustomModalDimensions()`. + * Only used by `getInitialSafeAreaConfig()` to predict safe-area + * requirements before the modal is presented. Callers that only use + * the context for non-initial paths can omit this. */ - hasCustomDimensions?: boolean; + coveredAxes?: ModalCoveredAxes; } -/** - * These thresholds match the SCSS media query breakpoints in modal.vars.scss - * that trigger the centered dialog layout (non-fullscreen modal). - * - * SCSS defines two height breakpoints: $modal-inset-min-height-small (600px) - * and $modal-inset-min-height-large (768px). We use the smaller one because - * that's the threshold where the modal transitions from fullscreen to centered - * dialog β€” the larger breakpoint only increases the dialog's height. - */ -const MODAL_INSET_MIN_WIDTH = 768; -const MODAL_INSET_MIN_HEIGHT = 600; const EDGE_THRESHOLD = 5; -/** - * CSS values for `--width` / `--height` that are treated as fullscreen - * (modal touches the corresponding screen edges). Empty string means the - * property was not overridden. See `hasCustomModalDimensions()`. - */ -const FULLSCREEN_SIZE_VALUES = new Set(['', '100%', '100vw', '100vh', '100dvw', '100dvh', '100svw', '100svh']); - /** * Cache for resolved root safe-area-top value, invalidated once per frame. */ let cachedRootSafeAreaTop: number | null = null; let cacheInvalidationScheduled = false; -/** - * Determines if the current viewport meets the CSS media query conditions - * that cause regular modals to render as centered dialogs instead of fullscreen. - * Matches: @media (min-width: 768px) and (min-height: 600px) - */ -const isCenteredDialogViewport = (): boolean => { - if (!win) return false; - return win.matchMedia(`(min-width: ${MODAL_INSET_MIN_WIDTH}px) and (min-height: ${MODAL_INSET_MIN_HEIGHT}px)`) - .matches; -}; - /** * Resolves the current root --ion-safe-area-top value to pixels. * Uses a temporary element because getComputedStyle on :root returns @@ -105,59 +90,72 @@ export const getRootSafeAreaTop = (): number => { }; /** - * Calls back when the resolved root `--ion-safe-area-top` changes, which no - * event and no window resize covers. The probe's height tracks the variable, so - * a change to it becomes a size change the observer can see. + * Calls back when the resolved root `--ion-safe-area-top` changes. The value + * the caller already applied is passed as the baseline, so a change between + * that read and the observer starting is still reported. */ export const onRootSafeAreaTopChange = (callback: (safeAreaTop: number) => void): (() => void) => { - const doc = win?.document; - if (!doc?.body || typeof ResizeObserver === 'undefined') { - return () => undefined; - } + return onCustomPropertyChange(win?.document?.body, '--ion-safe-area-top', callback, getRootSafeAreaTop()); +}; - const probe = doc.createElement('div'); - probe.style.cssText = - 'position:fixed;visibility:hidden;pointer-events:none;top:0;left:0;width:0;' + - 'height:var(--ion-safe-area-top,0px);'; - doc.body.appendChild(probe); +/** + * Determines which viewport axes the modal spans so safe-area requirements + * can be predicted independently for each axis. + * + * A modal that spans an axis reaches both edges on that axis and needs the + * corresponding safe-area insets. A modal that does not span an axis reaches + * neither edge on that axis. + * + * When both `--width` and `--height` are `fullscreen`, coverage can be + * determined directly. Otherwise, coverage is based on the rendered wrapper, + * including cases where content sizing or `--max-height` causes the modal + * to reach the viewport. + */ +export const getModalCoveredAxes = (hostEl: HTMLElement): ModalCoveredAxes => { + const styles = getComputedStyle(hostEl); + const width = getOverlaySizeType(styles.getPropertyValue('--width')); + const height = getOverlaySizeType(styles.getPropertyValue('--height')); - /** - * Seeded with the value the caller has already applied, so a change that - * lands before the observer's first delivery still gets reported. Comparing - * against an unset value instead would consume that first delivery and treat - * the new inset as the baseline. - */ - let lastHeight = getRootSafeAreaTop(); - const observer = new ResizeObserver((entries) => { - const { height } = entries[0].contentRect; - if (height !== lastHeight) { - lastHeight = height; - callback(height); - } - }); - observer.observe(probe); + if (width === 'fullscreen' && height === 'fullscreen') { + return { vertical: true, horizontal: true }; + } - return () => { - observer.disconnect(); - probe.remove(); - }; + return measureCoveredAxes(hostEl); }; /** - * True when the modal host declares BOTH a non-fullscreen `--width` AND a - * non-fullscreen `--height` (i.e. a centered-dialog-like modal that doesn't - * touch any screen edge). + * Measures the modal wrapper to determine whether it spans the viewport + * on each axis. + * + * The wrapper has no box while the modal is hidden, so `overlay-hidden` + * is temporarily removed to allow the wrapper to be measured. The class + * is restored in the same task before the browser can paint. * - * The conservative "both axes" check avoids mis-zeroing safe-area for - * partial-custom modals where the modal still touches top/bottom edges - * (e.g. only `--width` overridden). Partial cases fall through to the - * existing position-based post-animation correction. + * Only the wrapper's size is measured. Its position is affected by the + * enter animation, which initially translates it by its own height, while + * the translation does not affect its measured size. */ -export const hasCustomModalDimensions = (hostEl: HTMLElement): boolean => { - const styles = getComputedStyle(hostEl); - const width = styles.getPropertyValue('--width').trim(); - const height = styles.getPropertyValue('--height').trim(); - return !FULLSCREEN_SIZE_VALUES.has(width) && !FULLSCREEN_SIZE_VALUES.has(height); +const measureCoveredAxes = (hostEl: HTMLElement): ModalCoveredAxes => { + const wrapperEl = hostEl.shadowRoot?.querySelector('.modal-wrapper'); + if (wrapperEl == null || win === undefined) { + return { vertical: false, horizontal: false }; + } + + const wasHidden = hostEl.classList.contains('overlay-hidden'); + if (wasHidden) { + hostEl.classList.remove('overlay-hidden'); + } + + const { width, height } = wrapperEl.getBoundingClientRect(); + + if (wasHidden) { + hostEl.classList.add('overlay-hidden'); + } + + return { + vertical: height >= win.innerHeight - EDGE_THRESHOLD, + horizontal: width >= win.innerWidth - EDGE_THRESHOLD, + }; }; /** @@ -195,27 +193,24 @@ export const getInitialSafeAreaConfig = (context: ModalSafeAreaContext): SafeAre }; } - // On viewports that meet the centered dialog media query breakpoints, - // regular modals render as centered dialogs (not fullscreen), so they - // don't touch any screen edges and don't need safe-area insets. Also - // applies to phone viewports when the modal declares custom --width and - // --height; these don't touch screen edges either, so the initial - // prediction must be zero to avoid a post-animation correction flash. - if (isCenteredDialogViewport() || context.hasCustomDimensions) { - return { - top: '0px', - bottom: '0px', - left: '0px', - right: '0px', - }; - } + /** + * Each axis is evaluated independently because a modal can span one axis + * without spanning the other. This allows the initial safe-area configuration + * to match the modal's expected dimensions and avoids correcting an incorrect + * pair of insets after presentation. + * + * A modal can span the horizontal axis while remaining inset vertically, or + * span the vertical axis while remaining inset horizontally. Wide viewports + * can also render regular modals as centered dialogs, while content-sized + * modals may still be clamped to the viewport. + */ + const { vertical, horizontal } = context.coveredAxes ?? { vertical: true, horizontal: true }; - // Fullscreen modals on phone - inherit all safe areas return { - top: 'inherit', - bottom: 'inherit', - left: 'inherit', - right: 'inherit', + top: vertical ? 'inherit' : '0px', + bottom: vertical ? 'inherit' : '0px', + left: horizontal ? 'inherit' : '0px', + right: horizontal ? 'inherit' : '0px', }; }; diff --git a/core/src/components/modal/test/content-height/index.html b/core/src/components/modal/test/content-height/index.html new file mode 100644 index 00000000000..9cbef076967 --- /dev/null +++ b/core/src/components/modal/test/content-height/index.html @@ -0,0 +1,403 @@ + + + + + Modal - Content Height + + + + + + + + + + + + + + +
+ + + Modal - Content Height + + + + +

Content-based heights

+ + + + + +

Definite heights

+ + + + +

Overflowing content

+ + + +

Other content-based cases

+ + + + +

Known gaps

+ + + + + + fit-content + + + + + + + + + + + auto + + + + + + + + + + + min-content + + + + + + + + + + + max-content + + + + + + + + + + + + default height + + + + + + + + + + + 300px + + + + + + + + + + + 2000px + + + + + + + + + + + fit-content + + + + + + + + + + + fit-content, max-height + + + + + + + + + + + +

Modal header

+ + + + + + + Toggled height + + + + + + + + + + + + ::part(content) + + + + + + + +
+ + + + + diff --git a/core/src/components/modal/test/content-height/modal.e2e.ts b/core/src/components/modal/test/content-height/modal.e2e.ts new file mode 100644 index 00000000000..788a725562c --- /dev/null +++ b/core/src/components/modal/test/content-height/modal.e2e.ts @@ -0,0 +1,478 @@ +import { expect } from '@playwright/test'; +import type { E2EPage } from '@utils/test/playwright'; +import { configs, test } from '@utils/test/playwright'; + +const ISSUE = 'https://github.com/ionic-team/ionic-framework/issues/31149'; + +/** Height of the child inside `ion-content`, so sizing can be asserted exactly. */ +const CHILD_HEIGHT = 200; + +/** Taller than any viewport under test, to force the overflow cases. */ +const TALL_CHILD_HEIGHT = 2000; + +/** + * Delays the remount long enough to trigger a fresh evaluation, but not long + * enough for the modal's later safe-area write to clear the stale class. + */ +const REMOUNT_TIMEOUT = 100; + +/** + * `setContent` has animations enabled by default, so `toBeVisible()` resolves as + * the modal starts animating in and everything after it is measured + * mid-animation. This turns animations off for each modal. + */ +const DISABLE_ANIMATIONS = ``; + +const contentModal = (css = '', childHeight = CHILD_HEIGHT) => ` + ${DISABLE_ANIMATIONS} + ${css === '' ? '' : ``} + + + + Modal + + + +
height: ${childHeight}px
+ + +`; + +/** + * Nav pages have to be registered before `ion-nav` resolves its root, and the + * nav has to arrive through the modal's `component` delegate. An `ion-nav` + * slotted inline renders no pages at all. + */ +const navModal = (css = '') => ` + ${css === '' ? '' : ``} + + +`; + +const getContentHeight = async (page: E2EPage) => { + const box = await page.locator('ion-modal ion-content').first().boundingBox(); + return box?.height ?? 0; +}; + +const getWrapperHeight = async (page: E2EPage) => { + const box = await page.locator('ion-modal .modal-wrapper').boundingBox(); + return box?.height ?? 0; +}; + +/** + * A content-sized modal has no definite height to hand down, so the scroll + * container only scrolls if it can shrink against the modal's `--max-height`. + * `scrollHeight > clientHeight` is what separates scrolling from clipping. + */ +const getScrollMetrics = (page: E2EPage) => { + return page.locator('ion-modal ion-content').evaluate(async (el: HTMLIonContentElement) => { + const scrollEl = await el.getScrollElement(); + return { scrollHeight: scrollEl.scrollHeight, clientHeight: scrollEl.clientHeight }; + }); +}; + +/** + * Simulates a framework-driven detach/reattach around a modal height change: + * removes the content from the DOM, updates the modal's `--height` while the + * content is detached, then restores it to its original parent. + * + * The same element has to come back for this to reach the reconnect path, the + * way a framework moves a subtree it owns instead of rebuilding it, such as + * Vue's ``. Conditional rendering that discards the element and + * creates a new one is sized by that element's first render instead. + */ +const setHeightWhileDetached = (page: E2EPage, height: string) => { + return page.locator('ion-modal').evaluate(async (el: HTMLElement, height: string) => { + const content = el.querySelector('ion-content')!; + const parent = content.parentElement!; + + content.remove(); + el.style.setProperty('--height', height); + + await new Promise((resolve) => setTimeout(resolve, 200)); + parent.appendChild(content); + }, height); +}; + +/** Presents a nav modal through the delegate and waits for its first page. */ +const presentNavModal = async (page: E2EPage) => { + const ionModalDidPresent = await page.spyOnEvent('ionModalDidPresent'); + + await page.locator('ion-modal').evaluate((modal: HTMLIonModalElement) => { + modal.component = document.createElement('nav-host'); + return modal.present(); + }); + + await ionModalDidPresent.next(); + await page.locator('ion-modal ion-nav nav-page-one').waitFor(); +}; + +/** + * This behavior does not vary across directions + */ +configs({ directions: ['ltr'] }).forEach(({ title, screenshot, config }) => { + test.describe(title('modal: content height'), () => { + test.describe('content-based heights', () => { + /** + * Each of these leaves the content an indefinite height to resolve + * against, which is what used to collapse it. The content holds a single + * fixed height child, so a correct result is exactly that height: + * collapsed content measures 0, and a modal that ignored the height would + * fill the screen. + */ + const expectSizedToContent = async (page: E2EPage, height: string) => { + await page.setContent(contentModal(`ion-modal { --height: ${height}; }`), config); + await expect(page.locator('ion-modal')).toBeVisible(); + + await expect(page.locator('ion-modal ion-content')).toHaveClass(/content-sizing/); + await expect.poll(() => getContentHeight(page)).toBe(CHILD_HEIGHT); + }; + + test('should size the content with fit-content', async ({ page }) => { + test.info().annotations.push({ type: 'issue', description: ISSUE }); + + await expectSizedToContent(page, 'fit-content'); + }); + + test('should size the content with auto', async ({ page }) => { + test.info().annotations.push({ type: 'issue', description: ISSUE }); + + await expectSizedToContent(page, 'auto'); + }); + + test('should size the content with min-content', async ({ page }) => { + test.info().annotations.push({ type: 'issue', description: ISSUE }); + + await expectSizedToContent(page, 'min-content'); + }); + + test('should size the content with max-content', async ({ page }) => { + test.info().annotations.push({ type: 'issue', description: ISSUE }); + + await expectSizedToContent(page, 'max-content'); + }); + }); + + test.describe('definite heights', () => { + test('should fill the screen with the default height', async ({ page }) => { + await page.setContent(contentModal(), config); + await expect(page.locator('ion-modal')).toBeVisible(); + + const viewport = page.viewportSize()!; + + // Content sizing should not be applied by default. + await expect(page.locator('ion-modal ion-content')).not.toHaveClass(/content-sizing/); + await expect.poll(() => getWrapperHeight(page)).toBe(viewport.height); + }); + + test('should fill and scroll a pixel height', async ({ page }) => { + await page.setContent(contentModal('ion-modal { --height: 300px; }', TALL_CHILD_HEIGHT), config); + await expect(page.locator('ion-modal')).toBeVisible(); + + // A definite height is not content-sized, so the ion-content + // should fill the modal the way it always has. + await expect(page.locator('ion-modal ion-content')).not.toHaveClass(/content-sizing/); + await expect.poll(() => getWrapperHeight(page)).toBe(300); + + // The scroll container takes what the header leaves of the modal. + const headerHeight = (await page.locator('ion-modal ion-header').boundingBox())!.height; + const { scrollHeight, clientHeight } = await getScrollMetrics(page); + expect(clientHeight).toBe(300 - headerHeight); + expect(scrollHeight).toBeGreaterThan(clientHeight); + }); + + test('should clamp a pixel height taller than the overlay', async ({ page }) => { + await page.setContent(contentModal('ion-modal { --height: 2000px; }'), config); + await expect(page.locator('ion-modal')).toBeVisible(); + + const viewport = page.viewportSize()!; + + // 2000px exceeds the overlay, so the default --max-height: 100% should + // clamp the height rather than letting it run off screen. + await expect.poll(() => getWrapperHeight(page)).toBe(viewport.height); + }); + }); + + test.describe('overflowing content', () => { + test('should scroll rather than overflow the screen', async ({ page }) => { + await page.setContent(contentModal('ion-modal { --height: fit-content; }', TALL_CHILD_HEIGHT), config); + await expect(page.locator('ion-modal')).toBeVisible(); + + const viewport = page.viewportSize()!; + const headerHeight = (await page.locator('ion-modal ion-header').boundingBox())!.height; + + // The default --max-height keeps a content-sized modal inside the + // overlay, so overflowing content leaves it exactly as tall as the + // viewport rather than any height up to it. + await expect.poll(() => getWrapperHeight(page)).toBe(viewport.height); + + // The content takes what the header leaves and scrolls the child + // inside it, where a collapsed content would measure zero. + const { scrollHeight, clientHeight } = await getScrollMetrics(page); + expect(clientHeight).toBe(viewport.height - headerHeight); + expect(scrollHeight).toBeGreaterThan(clientHeight); + }); + + test('should honor a smaller --max-height', async ({ page }) => { + await page.setContent( + contentModal('ion-modal { --height: fit-content; --max-height: 50%; }', TALL_CHILD_HEIGHT), + config + ); + await expect(page.locator('ion-modal')).toBeVisible(); + + const viewport = page.viewportSize()!; + const headerHeight = (await page.locator('ion-modal ion-header').boundingBox())!.height; + + // Setting --max-height to 50% shrinks the modal to half the viewport. + // Half of an odd viewport lands on a sub-pixel, which the wrapper + // keeps and `clientHeight` rounds. + expect(await getWrapperHeight(page)).toBeCloseTo(viewport.height * 0.5, 0); + + // The content takes what the header leaves and scrolls the child + // inside it, where a collapsed content would measure zero. + const { scrollHeight, clientHeight } = await getScrollMetrics(page); + expect(clientHeight).toBe(Math.round(viewport.height * 0.5 - headerHeight)); + expect(scrollHeight).toBeGreaterThan(clientHeight); + }); + }); + + test.describe('structure and reactivity', () => { + test('should size a modal that has no ion-content', async ({ page }) => { + await page.setContent( + ` + ${DISABLE_ANIMATIONS} + + +
+ + `, + config + ); + await expect(page.locator('ion-modal')).toBeVisible(); + + // Sized through `ion-modal > .ion-page` alone, with none of the + // content-sizing detection involved. + await expect(page.locator('ion-modal ion-content')).toHaveCount(0); + await expect.poll(() => getWrapperHeight(page)).toBe(CHILD_HEIGHT); + }); + + test('should size a modal around an ion-nav and follow it between pages', async ({ page }) => { + await page.setContent(navModal('ion-modal { --height: fit-content; }'), config); + await presentNavModal(page); + + // Without the nav being positioned relatively it has no intrinsic + // height, so the modal would be 0. + const pageOneHeight = await getWrapperHeight(page); + expect(pageOneHeight).toBeGreaterThan(100); + + // Page two is taller, so the modal grows to follow the active page. + await page.locator('ion-modal ion-nav').evaluate((nav: HTMLIonNavElement) => nav.push('nav-page-two')); + await page.locator('ion-modal #tall-block').waitFor(); + + expect(await getWrapperHeight(page)).toBeGreaterThan(pageOneHeight); + }); + + test('should overlap nav pages mid-transition rather than stack them', async ({ page }) => { + /** + * The nav fixture keeps animations enabled so both pages are in the + * tree at once during the transition, which is what makes it possible + * to catch them laid out one below the other. + */ + await page.setContent(navModal('ion-modal { --height: fit-content; }'), config); + await presentNavModal(page); + + const tops = await page.locator('ion-modal ion-nav').evaluate(async (nav: HTMLIonNavElement) => { + const pushed = nav.push('nav-page-two'); + + /** + * Both pages are in the tree from the first frame of the transition, + * which runs for around half a second, so one frame is enough to + * catch them together. A page that has been hidden reports a zero + * rect, so only pages with a real box count. + */ + await new Promise((resolve) => requestAnimationFrame(resolve)); + const laidOut = Array.from(nav.children).filter((child) => child.getBoundingClientRect().height > 0); + const tops = laidOut.map((child) => Math.round(child.getBoundingClientRect().top)); + + // Awaiting the push surfaces a rejected transition as a test failure. + await pushed; + + return tops; + }); + + // Both pages are laid out during the slide and must share an origin. + expect(tops).toHaveLength(2); + expect(new Set(tops).size).toBe(1); + }); + + /** + * Nav pages carried these properties once before, at `height: 100%`, and + * it left titles animating to the wrong place (#25677, #25688). This + * covers where a transition ends up, with the arriving page and its title + * resting against the modal. + */ + test('should settle a nav transition with the new page in place', async ({ page }) => { + await page.setContent(navModal('ion-modal { --height: fit-content; }'), config); + await presentNavModal(page); + + // Awaiting the push resolves once the transition is done. + await page.locator('ion-modal ion-nav').evaluate((nav: HTMLIonNavElement) => nav.push('nav-page-two')); + + const arrived = page.locator('ion-modal nav-page-two'); + await expect(arrived.locator('ion-title')).toBeVisible(); + await expect(page.locator('ion-modal nav-page-one')).toBeHidden(); + + // A page left mid-slide still has a box, so the box has to line up with + // the modal on both axes for the transition to have actually landed. + const pageBox = (await arrived.boundingBox())!; + const wrapperBox = (await page.locator('ion-modal .modal-wrapper').boundingBox())!; + expect(pageBox.x).toBeCloseTo(wrapperBox.x, 0); + expect(pageBox.y).toBeCloseTo(wrapperBox.y, 0); + expect(pageBox.height).toBeGreaterThan(0); + + /** + * The title drifting down the viewport is the reported symptom, so the + * header has to sit at the top of the modal with the title inside it. + * Each mode insets the title by a different amount. + */ + const headerBox = (await arrived.locator('ion-header').boundingBox())!; + const titleBox = (await arrived.locator('ion-title').boundingBox())!; + expect(headerBox.y).toBeCloseTo(wrapperBox.y, 0); + expect(titleBox.y).toBeGreaterThanOrEqual(headerBox.y); + expect(titleBox.y + titleBox.height).toBeLessThanOrEqual(headerBox.y + headerBox.height + 1); + + // Going back has to land the same way, since the pop animates too. + await page.locator('ion-modal ion-nav').evaluate((nav: HTMLIonNavElement) => nav.pop()); + + await expect(page.locator('ion-modal nav-page-one ion-title')).toBeVisible(); + await expect(arrived).toBeHidden(); + expect((await page.locator('ion-modal nav-page-one').boundingBox())!.x).toBeCloseTo(wrapperBox.x, 0); + }); + + test('should respect a --height set on the modal at runtime', async ({ page }) => { + await page.setContent(contentModal(), config); + await expect(page.locator('ion-modal')).toBeVisible(); + + const viewport = page.viewportSize()!; + const modal = page.locator('ion-modal'); + const content = page.locator('ion-modal ion-content'); + + // No --height of its own, so the modal is on its default full height. + await expect(content).not.toHaveClass(/content-sizing/); + await expect.poll(() => getWrapperHeight(page)).toBe(viewport.height); + + // Set the --height and verify the observer is picking it up and + // adding the content-sizing class to the content. + await modal.evaluate((el: HTMLElement) => el.style.setProperty('--height', 'fit-content')); + await expect(content).toHaveClass(/content-sizing/); + await expect.poll(() => getContentHeight(page)).toBe(CHILD_HEIGHT); + + // Removing it falls back to the default, so a class left behind in + // either direction is caught. + await modal.evaluate((el: HTMLElement) => el.style.removeProperty('--height')); + await expect(content).not.toHaveClass(/content-sizing/); + await expect.poll(() => getWrapperHeight(page)).toBe(viewport.height); + }); + + test('should respect a --height that changed while the content was detached', async ({ page }) => { + await page.setContent(contentModal(), config); + await expect(page.locator('ion-modal')).toBeVisible(); + + const viewport = page.viewportSize()!; + const content = page.locator('ion-modal ion-content'); + + // Coming back to a content-based height should size the content to its + // child rather than collapse it. + await setHeightWhileDetached(page, 'fit-content'); + await expect(content).toHaveClass(/content-sizing/, { timeout: REMOUNT_TIMEOUT }); + await expect.poll(() => getContentHeight(page)).toBe(CHILD_HEIGHT); + + // Coming back to a definite height should fill the modal again, so a + // class left behind in either direction is caught. + await setHeightWhileDetached(page, '100%'); + await expect(content).not.toHaveClass(/content-sizing/, { timeout: REMOUNT_TIMEOUT }); + await expect.poll(() => getWrapperHeight(page)).toBe(viewport.height); + }); + + test('should respect a dynamically added body class that sets --height', async ({ page }) => { + await page.setContent(contentModal('body.custom-class ion-modal { --height: fit-content; }'), config); + await expect(page.locator('ion-modal')).toBeVisible(); + + const viewport = page.viewportSize()!; + const content = page.locator('ion-modal ion-content'); + + await expect(content).not.toHaveClass(/content-sizing/); + await expect.poll(() => getWrapperHeight(page)).toBe(viewport.height); + + await page.evaluate(() => document.body.classList.add('custom-class')); + + await expect(content).toHaveClass(/content-sizing/); + await expect.poll(() => getContentHeight(page)).toBe(CHILD_HEIGHT); + }); + }); + }); + + test.describe(title('modal: content height rendering'), () => { + test('should render a modal sized to its content', async ({ page }) => { + await page.setContent(contentModal('ion-modal { --height: fit-content; }'), config); + await expect(page.locator('ion-modal')).toBeVisible(); + + await expect(page).toHaveScreenshot(screenshot('modal-content-height-basic')); + }); + + test('should render a content-sized modal whose content overflows', async ({ page }) => { + await page.setContent(contentModal('ion-modal { --height: fit-content; }', TALL_CHILD_HEIGHT), config); + await expect(page.locator('ion-modal')).toBeVisible(); + + await expect(page).toHaveScreenshot(screenshot('modal-content-height-overflow')); + }); + + test('should render a content-sized modal with an ion-nav', async ({ page }) => { + await page.setContent(navModal('ion-modal { --height: fit-content; }'), config); + await presentNavModal(page); + + await expect(page).toHaveScreenshot(screenshot('modal-content-height-nav')); + }); + }); +}); diff --git a/core/src/components/modal/test/content-height/modal.e2e.ts-snapshots/modal-content-height-basic-ios-ltr-Mobile-Chrome-linux.png b/core/src/components/modal/test/content-height/modal.e2e.ts-snapshots/modal-content-height-basic-ios-ltr-Mobile-Chrome-linux.png new file mode 100644 index 00000000000..4b987641db4 Binary files /dev/null and b/core/src/components/modal/test/content-height/modal.e2e.ts-snapshots/modal-content-height-basic-ios-ltr-Mobile-Chrome-linux.png differ diff --git a/core/src/components/modal/test/content-height/modal.e2e.ts-snapshots/modal-content-height-basic-ios-ltr-Mobile-Firefox-linux.png b/core/src/components/modal/test/content-height/modal.e2e.ts-snapshots/modal-content-height-basic-ios-ltr-Mobile-Firefox-linux.png new file mode 100644 index 00000000000..f7908d0d9b6 Binary files /dev/null and b/core/src/components/modal/test/content-height/modal.e2e.ts-snapshots/modal-content-height-basic-ios-ltr-Mobile-Firefox-linux.png differ diff --git a/core/src/components/modal/test/content-height/modal.e2e.ts-snapshots/modal-content-height-basic-ios-ltr-Mobile-Safari-linux.png b/core/src/components/modal/test/content-height/modal.e2e.ts-snapshots/modal-content-height-basic-ios-ltr-Mobile-Safari-linux.png new file mode 100644 index 00000000000..2a35d7e437d Binary files /dev/null and b/core/src/components/modal/test/content-height/modal.e2e.ts-snapshots/modal-content-height-basic-ios-ltr-Mobile-Safari-linux.png differ diff --git a/core/src/components/modal/test/content-height/modal.e2e.ts-snapshots/modal-content-height-basic-md-ltr-Mobile-Chrome-linux.png b/core/src/components/modal/test/content-height/modal.e2e.ts-snapshots/modal-content-height-basic-md-ltr-Mobile-Chrome-linux.png new file mode 100644 index 00000000000..d826f723182 Binary files /dev/null and b/core/src/components/modal/test/content-height/modal.e2e.ts-snapshots/modal-content-height-basic-md-ltr-Mobile-Chrome-linux.png differ diff --git a/core/src/components/modal/test/content-height/modal.e2e.ts-snapshots/modal-content-height-basic-md-ltr-Mobile-Firefox-linux.png b/core/src/components/modal/test/content-height/modal.e2e.ts-snapshots/modal-content-height-basic-md-ltr-Mobile-Firefox-linux.png new file mode 100644 index 00000000000..654f6470aad Binary files /dev/null and b/core/src/components/modal/test/content-height/modal.e2e.ts-snapshots/modal-content-height-basic-md-ltr-Mobile-Firefox-linux.png differ diff --git a/core/src/components/modal/test/content-height/modal.e2e.ts-snapshots/modal-content-height-basic-md-ltr-Mobile-Safari-linux.png b/core/src/components/modal/test/content-height/modal.e2e.ts-snapshots/modal-content-height-basic-md-ltr-Mobile-Safari-linux.png new file mode 100644 index 00000000000..5f601dd4757 Binary files /dev/null and b/core/src/components/modal/test/content-height/modal.e2e.ts-snapshots/modal-content-height-basic-md-ltr-Mobile-Safari-linux.png differ diff --git a/core/src/components/modal/test/content-height/modal.e2e.ts-snapshots/modal-content-height-nav-ios-ltr-Mobile-Chrome-linux.png b/core/src/components/modal/test/content-height/modal.e2e.ts-snapshots/modal-content-height-nav-ios-ltr-Mobile-Chrome-linux.png new file mode 100644 index 00000000000..eb8d2a12ff0 Binary files /dev/null and b/core/src/components/modal/test/content-height/modal.e2e.ts-snapshots/modal-content-height-nav-ios-ltr-Mobile-Chrome-linux.png differ diff --git a/core/src/components/modal/test/content-height/modal.e2e.ts-snapshots/modal-content-height-nav-ios-ltr-Mobile-Firefox-linux.png b/core/src/components/modal/test/content-height/modal.e2e.ts-snapshots/modal-content-height-nav-ios-ltr-Mobile-Firefox-linux.png new file mode 100644 index 00000000000..69a17bc29d5 Binary files /dev/null and b/core/src/components/modal/test/content-height/modal.e2e.ts-snapshots/modal-content-height-nav-ios-ltr-Mobile-Firefox-linux.png differ diff --git a/core/src/components/modal/test/content-height/modal.e2e.ts-snapshots/modal-content-height-nav-ios-ltr-Mobile-Safari-linux.png b/core/src/components/modal/test/content-height/modal.e2e.ts-snapshots/modal-content-height-nav-ios-ltr-Mobile-Safari-linux.png new file mode 100644 index 00000000000..912aaec64b5 Binary files /dev/null and b/core/src/components/modal/test/content-height/modal.e2e.ts-snapshots/modal-content-height-nav-ios-ltr-Mobile-Safari-linux.png differ diff --git a/core/src/components/modal/test/content-height/modal.e2e.ts-snapshots/modal-content-height-nav-md-ltr-Mobile-Chrome-linux.png b/core/src/components/modal/test/content-height/modal.e2e.ts-snapshots/modal-content-height-nav-md-ltr-Mobile-Chrome-linux.png new file mode 100644 index 00000000000..fe0a14a3bba Binary files /dev/null and b/core/src/components/modal/test/content-height/modal.e2e.ts-snapshots/modal-content-height-nav-md-ltr-Mobile-Chrome-linux.png differ diff --git a/core/src/components/modal/test/content-height/modal.e2e.ts-snapshots/modal-content-height-nav-md-ltr-Mobile-Firefox-linux.png b/core/src/components/modal/test/content-height/modal.e2e.ts-snapshots/modal-content-height-nav-md-ltr-Mobile-Firefox-linux.png new file mode 100644 index 00000000000..908a72af971 Binary files /dev/null and b/core/src/components/modal/test/content-height/modal.e2e.ts-snapshots/modal-content-height-nav-md-ltr-Mobile-Firefox-linux.png differ diff --git a/core/src/components/modal/test/content-height/modal.e2e.ts-snapshots/modal-content-height-nav-md-ltr-Mobile-Safari-linux.png b/core/src/components/modal/test/content-height/modal.e2e.ts-snapshots/modal-content-height-nav-md-ltr-Mobile-Safari-linux.png new file mode 100644 index 00000000000..9430111477d Binary files /dev/null and b/core/src/components/modal/test/content-height/modal.e2e.ts-snapshots/modal-content-height-nav-md-ltr-Mobile-Safari-linux.png differ diff --git a/core/src/components/modal/test/content-height/modal.e2e.ts-snapshots/modal-content-height-overflow-ios-ltr-Mobile-Chrome-linux.png b/core/src/components/modal/test/content-height/modal.e2e.ts-snapshots/modal-content-height-overflow-ios-ltr-Mobile-Chrome-linux.png new file mode 100644 index 00000000000..ede48236109 Binary files /dev/null and b/core/src/components/modal/test/content-height/modal.e2e.ts-snapshots/modal-content-height-overflow-ios-ltr-Mobile-Chrome-linux.png differ diff --git a/core/src/components/modal/test/content-height/modal.e2e.ts-snapshots/modal-content-height-overflow-ios-ltr-Mobile-Firefox-linux.png b/core/src/components/modal/test/content-height/modal.e2e.ts-snapshots/modal-content-height-overflow-ios-ltr-Mobile-Firefox-linux.png new file mode 100644 index 00000000000..39a951afbcc Binary files /dev/null and b/core/src/components/modal/test/content-height/modal.e2e.ts-snapshots/modal-content-height-overflow-ios-ltr-Mobile-Firefox-linux.png differ diff --git a/core/src/components/modal/test/content-height/modal.e2e.ts-snapshots/modal-content-height-overflow-ios-ltr-Mobile-Safari-linux.png b/core/src/components/modal/test/content-height/modal.e2e.ts-snapshots/modal-content-height-overflow-ios-ltr-Mobile-Safari-linux.png new file mode 100644 index 00000000000..a4c44d457b9 Binary files /dev/null and b/core/src/components/modal/test/content-height/modal.e2e.ts-snapshots/modal-content-height-overflow-ios-ltr-Mobile-Safari-linux.png differ diff --git a/core/src/components/modal/test/content-height/modal.e2e.ts-snapshots/modal-content-height-overflow-md-ltr-Mobile-Chrome-linux.png b/core/src/components/modal/test/content-height/modal.e2e.ts-snapshots/modal-content-height-overflow-md-ltr-Mobile-Chrome-linux.png new file mode 100644 index 00000000000..72ab1dc9a83 Binary files /dev/null and b/core/src/components/modal/test/content-height/modal.e2e.ts-snapshots/modal-content-height-overflow-md-ltr-Mobile-Chrome-linux.png differ diff --git a/core/src/components/modal/test/content-height/modal.e2e.ts-snapshots/modal-content-height-overflow-md-ltr-Mobile-Firefox-linux.png b/core/src/components/modal/test/content-height/modal.e2e.ts-snapshots/modal-content-height-overflow-md-ltr-Mobile-Firefox-linux.png new file mode 100644 index 00000000000..9aeb4618795 Binary files /dev/null and b/core/src/components/modal/test/content-height/modal.e2e.ts-snapshots/modal-content-height-overflow-md-ltr-Mobile-Firefox-linux.png differ diff --git a/core/src/components/modal/test/content-height/modal.e2e.ts-snapshots/modal-content-height-overflow-md-ltr-Mobile-Safari-linux.png b/core/src/components/modal/test/content-height/modal.e2e.ts-snapshots/modal-content-height-overflow-md-ltr-Mobile-Safari-linux.png new file mode 100644 index 00000000000..239e39bff1c Binary files /dev/null and b/core/src/components/modal/test/content-height/modal.e2e.ts-snapshots/modal-content-height-overflow-md-ltr-Mobile-Safari-linux.png differ diff --git a/core/src/components/modal/test/safe-area/index.html b/core/src/components/modal/test/safe-area/index.html index 14681f3820f..f5b1f55d57d 100644 --- a/core/src/components/modal/test/safe-area/index.html +++ b/core/src/components/modal/test/safe-area/index.html @@ -71,6 +71,20 @@

Card Modals (iOS)

Centered Dialog (Tablet)

+

Content Sized Dialog

+ + + + +

Diagnostic Info

Window Width:

@@ -228,6 +242,55 @@

Modal Safe-Area Overrides:

modal.remove(); } + /** + * A dialog with custom dimensions on both axes and a content-sized + * height. Short content keeps it away from the screen edges, while + * overflowing content causes `--max-height` to clamp it to the + * viewport, where it reaches the top and bottom edges. + */ + async function presentContentSizedDialog(childHeight, width = '300px') { + const element = document.createElement('div'); + element.innerHTML = ` + + + Content Sized Dialog + + Close + + + + +

Content sized dialog.

+
+

Last line of content, which the home indicator must not cover.

+ + `; + + const modal = Object.assign(document.createElement('ion-modal'), { + component: element, + cssClass: 'content-sized-dialog', + }); + + const style = document.createElement('style'); + style.textContent = ` + .content-sized-dialog { + --width: ${width}; + --height: fit-content; + } + `; + document.head.appendChild(style); + + element.querySelector('.dismiss').addEventListener('click', () => modal.dismiss()); + document.body.appendChild(modal); + + await modal.present(); + updateModalDiagnostics(modal); + + await modal.onDidDismiss(); + modal.remove(); + style.remove(); + } + async function presentCenteredDialog() { const element = createModalContent('Centered Dialog'); // Centered dialog uses custom dimensions diff --git a/core/src/components/modal/test/safe-area/modal.e2e.ts b/core/src/components/modal/test/safe-area/modal.e2e.ts index 4905f645aa3..740eb83c298 100644 --- a/core/src/components/modal/test/safe-area/modal.e2e.ts +++ b/core/src/components/modal/test/safe-area/modal.e2e.ts @@ -1,5 +1,6 @@ import { expect } from '@playwright/test'; import type { Locator } from '@playwright/test'; +import type { E2EPage } from '@utils/test/playwright'; import { configs, detachAndReattach, test, Viewports } from '@utils/test/playwright'; /** @@ -25,6 +26,49 @@ configs({ modes: ['ios', 'md'], directions: ['ltr'] }).forEach(({ title, config await page.goto('/src/components/modal/test/safe-area', config); }); + /** + * The safe-area prediction is applied before the modal is shown, so + * reading it when the modal starts presenting captures the prediction + * before the position-based correction runs. + */ + const getPredictedSafeArea = async (page: E2EPage, trigger: string) => { + await page.evaluate(() => { + document.addEventListener( + 'ionModalWillPresent', + (ev) => { + const modal = ev.target as HTMLElement; + (window as any).predictedSafeArea = { + top: modal.style.getPropertyValue('--ion-safe-area-top'), + bottom: modal.style.getPropertyValue('--ion-safe-area-bottom'), + left: modal.style.getPropertyValue('--ion-safe-area-left'), + right: modal.style.getPropertyValue('--ion-safe-area-right'), + }; + }, + { once: true } + ); + }); + + const ionModalDidPresent = await page.spyOnEvent('ionModalDidPresent'); + await page.click(trigger); + await ionModalDidPresent.next(); + + return page.evaluate(() => (window as any).predictedSafeArea); + }; + + /** + * The safe-area values after the modal has finished presenting. + * These reflect the modal's actual position and are the values that + * the initial prediction should converge to. + */ + const getSettledSafeArea = (page: E2EPage) => { + return page.locator('ion-modal').evaluate((el: HTMLElement) => ({ + top: el.style.getPropertyValue('--ion-safe-area-top'), + bottom: el.style.getPropertyValue('--ion-safe-area-bottom'), + left: el.style.getPropertyValue('--ion-safe-area-left'), + right: el.style.getPropertyValue('--ion-safe-area-right'), + })); + }; + test('fullscreen modal should inherit all safe-area values on phone', async ({ page }, testInfo) => { testInfo.annotations.push({ type: 'issue', @@ -50,7 +94,20 @@ configs({ modes: ['ios', 'md'], directions: ['ltr'] }).forEach(({ title, config expect(safeAreaBottom).toBe('inherit'); }); - test('regular modal should have safe-area zeroed on tablet (centered dialog)', async ({ page }, testInfo) => { + test('regular modal should predict zeroed safe-area on tablet (centered dialog)', async ({ page }) => { + // The viewport gives it centered dialog dimensions, so it stays clear + // of every edge. + await page.setViewportSize(Viewports.tablet.portrait); + + expect(await getPredictedSafeArea(page, '#fullscreen-modal')).toEqual({ + top: '0px', + bottom: '0px', + left: '0px', + right: '0px', + }); + }); + + test('regular modal should have zeroed safe-area on tablet (centered dialog)', async ({ page }, testInfo) => { testInfo.annotations.push({ type: 'issue', description: 'https://github.com/ionic-team/ionic-framework/issues/30900', @@ -445,6 +502,94 @@ configs({ modes: ['ios', 'md'], directions: ['ltr'] }).forEach(({ title, config await modal.evaluate((el: HTMLIonModalElement) => el.remove()); }); + test.describe('content sized dialogs', () => { + test('should predict a zeroed safe-area for a dialog that fits its content', async ({ page }) => { + expect(await getPredictedSafeArea(page, '#content-sized-dialog')).toEqual({ + top: '0px', + bottom: '0px', + left: '0px', + right: '0px', + }); + }); + + /** + * Overflowing content causes the dialog to be clamped to the viewport + * and reach the top and bottom edges, so the insets must be applied + * from the first frame. Predicting zero here would cause the header + * to change height once the modal has finished presenting. + */ + test('should predict an inherited safe-area for a dialog whose content overflows', async ({ page }) => { + expect(await getPredictedSafeArea(page, '#content-sized-dialog-tall')).toEqual({ + top: 'inherit', + bottom: 'inherit', + left: '0px', + right: '0px', + }); + }); + + /** + * A full-width dialog reaches the horizontal edges while staying clear + * of the top and bottom, so the safe-area values differ by edge. + */ + test('should predict per edge for a full width dialog that fits its content', async ({ page }) => { + expect(await getPredictedSafeArea(page, '#content-sized-dialog-full-width')).toEqual({ + top: '0px', + bottom: '0px', + left: 'inherit', + right: 'inherit', + }); + }); + + test('should predict an inherited safe-area for a full width dialog that overflows', async ({ page }) => { + expect(await getPredictedSafeArea(page, '#content-sized-dialog-full-width-tall')).toEqual({ + top: 'inherit', + bottom: 'inherit', + left: 'inherit', + right: 'inherit', + }); + }); + + /** + * A wide viewport gives regular modals the dimensions of a centered + * dialog, but a modal sized to its content can still be clamped to the + * viewport by `--max-height` and reach the edges. + */ + test('should predict per axis on a wide viewport', async ({ page }) => { + await page.setViewportSize(Viewports.tablet.portrait); + + expect(await getPredictedSafeArea(page, '#content-sized-dialog-tall')).toEqual({ + top: 'inherit', + bottom: 'inherit', + left: '0px', + right: '0px', + }); + }); + + test('should predict an inherited safe-area for a full width overflowing dialog on a wide viewport', async ({ + page, + }) => { + await page.setViewportSize(Viewports.tablet.portrait); + + expect(await getPredictedSafeArea(page, '#content-sized-dialog-full-width-tall')).toEqual({ + top: 'inherit', + bottom: 'inherit', + left: 'inherit', + right: 'inherit', + }); + }); + + /** + * A position-based pass replaces the prediction once the modal has + * presented. Any edge where the two disagree changes value at that + * point, which can cause the header to grow or shrink. + */ + test('should predict what the modal settles on', async ({ page }) => { + const predicted = await getPredictedSafeArea(page, '#content-sized-dialog-full-width'); + + expect(predicted).toEqual(await getSettledSafeArea(page)); + }); + }); + test.describe('moving a presented modal', () => { const moveModal = (modal: Locator) => detachAndReattach(modal, 'ion-app'); diff --git a/core/src/components/modal/utils.ts b/core/src/components/modal/utils.ts index ac01b3eebe8..ba426c1f186 100644 --- a/core/src/components/modal/utils.ts +++ b/core/src/components/modal/utils.ts @@ -1,4 +1,5 @@ import { win } from '@utils/browser'; +import { onCustomPropertyChange } from '@utils/helpers'; import { StatusBar, Style } from '@utils/native/status-bar'; /** @@ -84,3 +85,10 @@ export const setCardStatusBarDefault = (defaultStyle = Style.Default) => { StatusBar.setStyle({ style: defaultStyle }); }; + +/** + * Calls back when the modal's resolved `--height` changes. + */ +export const onModalHeightChange = (hostEl: HTMLElement, callback: () => void): (() => void) => { + return onCustomPropertyChange(hostEl, '--height', () => callback()); +}; diff --git a/core/src/components/popover/animations/ios.enter.ts b/core/src/components/popover/animations/ios.enter.ts index 02a078a2f71..4273074f6bb 100644 --- a/core/src/components/popover/animations/ios.enter.ts +++ b/core/src/components/popover/animations/ios.enter.ts @@ -5,6 +5,7 @@ import type { Animation } from '../../../interface'; import { calculateWindowAdjustment, getArrowDimensions, + getElementCSSZoom, getPopoverDimensions, getPopoverPosition, getSafeAreaInsets, @@ -31,16 +32,31 @@ export const iosEnterAnimation = (baseEl: HTMLElement, opts?: any): Animation => const { event: ev, size, trigger, reference, side, align } = opts; const doc = baseEl.ownerDocument as any; const isRTL = doc.dir === 'rtl'; - const bodyWidth = doc.defaultView.innerWidth; - const bodyHeight = doc.defaultView.innerHeight; - const root = getElementRoot(baseEl); const contentEl = root.querySelector('.popover-content') as HTMLElement; const arrowEl = root.querySelector('.popover-arrow') as HTMLElement | null; + /** + * A CSS `zoom` other than 1 on an ancestor (e.g. the `html` element) causes + * geometry APIs like `getBoundingClientRect()` to report zoomed values while + * inline `top`/`left`/`--width` styles are interpreted in the unzoomed layout + * space. Normalize all rect-derived measurements by this factor so the + * popover is positioned and sized correctly. + */ + const zoom = getElementCSSZoom(contentEl); + + /** + * `innerWidth`/`innerHeight` are not affected by CSS `zoom`, so they must be + * scaled down to the same layout space as the normalized measurements above. + * Otherwise the popover would be clamped against a viewport that is larger + * than the space actually available to it. + */ + const bodyWidth = doc.defaultView.innerWidth / zoom; + const bodyHeight = doc.defaultView.innerHeight / zoom; + const referenceSizeEl = trigger || ev?.detail?.ionShadowTarget || ev?.target; - const { contentWidth, contentHeight } = getPopoverDimensions(size, contentEl, referenceSizeEl); - const { arrowWidth, arrowHeight } = getArrowDimensions(arrowEl); + const { contentWidth, contentHeight } = getPopoverDimensions(size, contentEl, referenceSizeEl, zoom); + const { arrowWidth, arrowHeight } = getArrowDimensions(arrowEl, zoom); const defaultPosition = { top: bodyHeight / 2 - contentHeight / 2, @@ -60,7 +76,8 @@ export const iosEnterAnimation = (baseEl: HTMLElement, opts?: any): Animation => align, defaultPosition, trigger, - ev + ev, + zoom ); const padding = size === 'cover' ? 0 : POPOVER_IOS_BODY_PADDING; diff --git a/core/src/components/popover/animations/md.enter.ts b/core/src/components/popover/animations/md.enter.ts index 8de9976e86c..6d37474ceb2 100644 --- a/core/src/components/popover/animations/md.enter.ts +++ b/core/src/components/popover/animations/md.enter.ts @@ -2,7 +2,13 @@ import { createAnimation } from '@utils/animation/animation'; import { getElementRoot } from '@utils/helpers'; import type { Animation } from '../../../interface'; -import { calculateWindowAdjustment, getPopoverDimensions, getPopoverPosition, getSafeAreaInsets } from '../utils'; +import { + calculateWindowAdjustment, + getElementCSSZoom, + getPopoverDimensions, + getPopoverPosition, + getSafeAreaInsets, +} from '../utils'; const POPOVER_MD_BODY_PADDING = 12; @@ -15,14 +21,29 @@ export const mdEnterAnimation = (baseEl: HTMLElement, opts?: any): Animation => const doc = baseEl.ownerDocument as any; const isRTL = doc.dir === 'rtl'; - const bodyWidth = doc.defaultView.innerWidth; - const bodyHeight = doc.defaultView.innerHeight; - const root = getElementRoot(baseEl); const contentEl = root.querySelector('.popover-content') as HTMLElement; + /** + * A CSS `zoom` other than 1 on an ancestor (e.g. the `html` element) causes + * geometry APIs like `getBoundingClientRect()` to report zoomed values while + * inline `top`/`left`/`--width` styles are interpreted in the unzoomed layout + * space. Normalize all rect-derived measurements by this factor so the + * popover is positioned and sized correctly. + */ + const zoom = getElementCSSZoom(contentEl); + + /** + * `innerWidth`/`innerHeight` are not affected by CSS `zoom`, so they must be + * scaled down to the same layout space as the normalized measurements above. + * Otherwise the popover would be clamped against a viewport that is larger + * than the space actually available to it. + */ + const bodyWidth = doc.defaultView.innerWidth / zoom; + const bodyHeight = doc.defaultView.innerHeight / zoom; + const referenceSizeEl = trigger || ev?.detail?.ionShadowTarget || ev?.target; - const { contentWidth, contentHeight } = getPopoverDimensions(size, contentEl, referenceSizeEl); + const { contentWidth, contentHeight } = getPopoverDimensions(size, contentEl, referenceSizeEl, zoom); const defaultPosition = { top: bodyHeight / 2 - contentHeight / 2, @@ -42,7 +63,8 @@ export const mdEnterAnimation = (baseEl: HTMLElement, opts?: any): Animation => align, defaultPosition, trigger, - ev + ev, + zoom ); const padding = size === 'cover' ? 0 : POPOVER_MD_BODY_PADDING; diff --git a/core/src/components/popover/test/util.spec.ts b/core/src/components/popover/test/util.spec.ts index a383209f96c..cda094f39c6 100644 --- a/core/src/components/popover/test/util.spec.ts +++ b/core/src/components/popover/test/util.spec.ts @@ -1,4 +1,89 @@ -import { isTriggerElement, getIndexOfItem, getNextItem, getPrevItem } from '../utils'; +import { + isTriggerElement, + getIndexOfItem, + getNextItem, + getPrevItem, + getElementCSSZoom, + getPopoverDimensions, + getArrowDimensions, +} from '../utils'; + +describe('getElementCSSZoom', () => { + it('should return 1 when no element is provided', () => { + expect(getElementCSSZoom(null)).toEqual(1); + }); + + it('should use currentCSSZoom when available', () => { + const el = document.createElement('div'); + Object.defineProperty(el, 'currentCSSZoom', { value: 1.5, configurable: true }); + + expect(getElementCSSZoom(el)).toEqual(1.5); + }); + + it('should fall back to the ratio between the client rect and offsetWidth', () => { + const el = document.createElement('div'); + // No currentCSSZoom support in this environment. + el.getBoundingClientRect = () => ({ width: 300, height: 0, top: 0, left: 0, bottom: 0, right: 0 } as DOMRect); + Object.defineProperty(el, 'offsetWidth', { value: 200, configurable: true }); + + expect(getElementCSSZoom(el)).toEqual(1.5); + }); + + it('should treat sub-pixel rounding in the fallback as no zoom', () => { + const el = document.createElement('div'); + // offsetWidth is rounded to an integer, the bounding rect is not. + el.getBoundingClientRect = () => ({ width: 250.4, height: 0, top: 0, left: 0, bottom: 0, right: 0 } as DOMRect); + Object.defineProperty(el, 'offsetWidth', { value: 250, configurable: true }); + + expect(getElementCSSZoom(el)).toEqual(1); + }); + + it('should return 1 when the fallback measurements are unavailable', () => { + const el = document.createElement('div'); + el.getBoundingClientRect = () => ({ width: 0, height: 0, top: 0, left: 0, bottom: 0, right: 0 } as DOMRect); + Object.defineProperty(el, 'offsetWidth', { value: 0, configurable: true }); + + expect(getElementCSSZoom(el)).toEqual(1); + }); +}); + +describe('getPopoverDimensions', () => { + it('should normalize the content dimensions by the zoom factor', () => { + const contentEl = document.createElement('div'); + contentEl.getBoundingClientRect = () => + ({ width: 300, height: 450, top: 0, left: 0, bottom: 0, right: 0 } as DOMRect); + + const { contentWidth, contentHeight } = getPopoverDimensions('auto', contentEl, undefined, 1.5); + + expect(contentWidth).toEqual(200); + expect(contentHeight).toEqual(300); + }); + + it('should normalize the trigger width by the zoom factor when size is cover', () => { + const contentEl = document.createElement('div'); + contentEl.getBoundingClientRect = () => + ({ width: 300, height: 450, top: 0, left: 0, bottom: 0, right: 0 } as DOMRect); + const triggerEl = document.createElement('div'); + triggerEl.getBoundingClientRect = () => + ({ width: 150, height: 60, top: 0, left: 0, bottom: 0, right: 0 } as DOMRect); + + const { contentWidth } = getPopoverDimensions('cover', contentEl, triggerEl, 1.5); + + expect(contentWidth).toEqual(100); + }); +}); + +describe('getArrowDimensions', () => { + it('should normalize the arrow dimensions by the zoom factor', () => { + const arrowEl = document.createElement('div'); + arrowEl.getBoundingClientRect = () => ({ width: 15, height: 15, top: 0, left: 0, bottom: 0, right: 0 } as DOMRect); + + const { arrowWidth, arrowHeight } = getArrowDimensions(arrowEl, 1.5); + + expect(arrowWidth).toEqual(10); + expect(arrowHeight).toEqual(10); + }); +}); describe('isTriggerElement', () => { it('should return true is element is a trigger', () => { diff --git a/core/src/components/popover/test/zoom/index.html b/core/src/components/popover/test/zoom/index.html new file mode 100644 index 00000000000..96a7488768d --- /dev/null +++ b/core/src/components/popover/test/zoom/index.html @@ -0,0 +1,76 @@ + + + + + Popover - Zoom + + + + + + + + + + + + + + Popover - Zoom + + + + + + + Auto + + + + + Cover + + + + + Edge + + + + + diff --git a/core/src/components/popover/test/zoom/popover.e2e.ts b/core/src/components/popover/test/zoom/popover.e2e.ts new file mode 100644 index 00000000000..beed5faa776 --- /dev/null +++ b/core/src/components/popover/test/zoom/popover.e2e.ts @@ -0,0 +1,315 @@ +import { expect } from '@playwright/test'; +import type { E2EPage } from '@utils/test/playwright'; +import { configs, test } from '@utils/test/playwright'; + +import { openPopover } from '../test.utils'; + +/** + * A CSS `zoom` causes geometry APIs such as `getBoundingClientRect()` and + * pointer `clientX`/`clientY` to report values in the zoomed coordinate space, + * while the inline `top`/`left`/`--width` styles the popover sets are + * interpreted in the unzoomed layout space. The popover needs to account for + * this so it stays anchored to its trigger. + * + * These are functional assertions rather than screenshots because what is being + * verified is the popover's geometry relative to its trigger, not its + * appearance. Both boxes are read in the same coordinate space, so the + * relationship between them holds at any zoom level. + */ + +/** + * Maximum difference, in pixels, between two positions still considered + * aligned. Generous enough for sub-pixel rounding across browsers, far tighter + * than the error a missing zoom adjustment produces (tens of pixels). + */ +const TOLERANCE = 2; + +const expectAligned = (actual: number, expected: number) => { + expect(Math.abs(actual - expected)).toBeLessThanOrEqual(TOLERANCE); +}; + +/** + * Builds a page with a trigger and a popover, with `zoomStyles` controlling + * where in the tree the zoom is applied. The trigger is kept near the top left + * so the popover is never pushed onto the screen by the offscreen adjustment, + * which would mask a positioning error. + */ +const zoomedPage = (zoomStyles: string) => ` + + + + + Content + +`; + +/** + * Markup where the zoom wraps only the trigger, leaving the popover outside the + * zoomed subtree. This is the split `popoverController.create()` produces by + * default, since the overlay is appended to `ion-app`. + */ +const triggerOnlyZoomPage = ` + + +
+ +
+ + Content + +`; + +/** + * The vertical sides take a larger zoom. The gap a missing `arrowHeight` + * normalization opens between the arrow and the content edge scales with it, + * so a larger factor keeps that gap comfortably clear of the tolerance. Going + * much beyond this leaves too little layout height below the trigger and the + * popover flips above it, which changes which edge the arrow sits against. + */ +const VERTICAL_SIDE_ZOOM = 1.5; + +/** + * The horizontal sides need a smaller one: at a larger zoom the popover no + * longer fits beside the trigger, and the offscreen adjustment would move it + * and mask the arrow position under test. + */ +const HORIZONTAL_SIDE_ZOOM = 1.25; + +/** + * Builds a page with the popover on a given side, under a zoom. The trigger sits + * in the middle so the popover fits on every side without the offscreen + * adjustment moving it, which would mask an arrow positioning error. + */ +const zoomedSidePage = (side: string, zoom: number) => ` + + + + + Content + +`; + +const expectAnchoredToTrigger = async (page: E2EPage) => { + const triggerBox = (await page.locator('#trigger').boundingBox())!; + const contentBox = (await page.locator('ion-popover').locator('.popover-content').boundingBox())!; + + expectAligned(contentBox.x, triggerBox.x); + expectAligned(contentBox.y, triggerBox.y + triggerBox.height); +}; + +/** + * This behavior does not vary across directions. MD mode is used because it has + * no arrow offsetting the content and defaults to `start` alignment, which + * makes the expected relationship to the trigger unambiguous. + */ +configs({ modes: ['md'], directions: ['ltr'] }).forEach(({ title, config }) => { + test.describe(title('popover: zoom'), () => { + test.beforeEach(() => { + test.info().annotations.push({ + type: 'issue', + description: 'https://github.com/ionic-team/ionic-framework/issues/30919', + }); + }); + + test.describe('zoom on the html element', () => { + test.beforeEach(async ({ page }) => { + await page.goto('/src/components/popover/test/zoom', config); + }); + + test('should align the popover with its trigger', async ({ page }) => { + await openPopover(page, 'auto-trigger'); + + const triggerBox = (await page.locator('#auto-trigger').boundingBox())!; + const contentBox = (await page.locator('ion-popover.auto-popover').locator('.popover-content').boundingBox())!; + + expectAligned(contentBox.x, triggerBox.x); + expectAligned(contentBox.y, triggerBox.y + triggerBox.height); + }); + + test('should not render the popover offscreen', async ({ page }) => { + await openPopover(page, 'edge-trigger'); + + const viewport = page.viewportSize()!; + const contentBox = (await page.locator('ion-popover.edge-popover').locator('.popover-content').boundingBox())!; + + expect(contentBox.x).toBeGreaterThanOrEqual(0); + expect(contentBox.x + contentBox.width).toBeLessThanOrEqual(viewport.width); + }); + + test('should match the trigger width when size is cover', async ({ page }) => { + await openPopover(page, 'cover-trigger'); + + const triggerBox = (await page.locator('#cover-trigger').boundingBox())!; + const contentBox = (await page.locator('ion-popover.cover-popover').locator('.popover-content').boundingBox())!; + + expectAligned(contentBox.width, triggerBox.width); + }); + }); + + /** + * The zoom must be read from the popover's own context rather than from + * `document.documentElement`, otherwise a zoom applied lower in the tree is + * missed entirely. + */ + test.describe('zoom applied at other levels of the tree', () => { + test('should align the popover when zoom is on the body', async ({ page }) => { + await page.setContent(zoomedPage('body { zoom: 1.5; }'), config); + await openPopover(page, 'trigger'); + + await expectAnchoredToTrigger(page); + }); + + test('should align the popover when zoom accumulates across ancestors', async ({ page }) => { + await page.setContent(zoomedPage('html { zoom: 1.2; } body { zoom: 1.25; }'), config); + await openPopover(page, 'trigger'); + + await expectAnchoredToTrigger(page); + }); + + test('should align the popover when the zoom wraps only the trigger', async ({ page }) => { + await page.setContent(triggerOnlyZoomPage, config); + await openPopover(page, 'trigger'); + + await expectAnchoredToTrigger(page); + }); + + test('should align the popover when the page is zoomed out', async ({ page }) => { + await page.setContent(zoomedPage('html { zoom: 0.8; }'), config); + await openPopover(page, 'trigger'); + + await expectAnchoredToTrigger(page); + }); + }); + + /** + * `reference="event"` positions the popover from the pointer coordinates of + * the event, which are reported in the zoomed coordinate space too. + */ + test.describe('pointer coordinates', () => { + test('should position the popover at the pointer when reference is event', async ({ page }) => { + await page.setContent( + zoomedPage('html { zoom: 1.5; }').replace('trigger="trigger"', 'trigger="trigger" reference="event"'), + config + ); + + const triggerBox = (await page.locator('#trigger').boundingBox())!; + await openPopover(page, 'trigger'); + + const contentBox = (await page.locator('ion-popover').locator('.popover-content').boundingBox())!; + + /** + * Playwright clicks the center of the trigger, which is where the + * popover should be anchored. + */ + expectAligned(contentBox.x, triggerBox.x + triggerBox.width / 2); + expectAligned(contentBox.y, triggerBox.y + triggerBox.height / 2); + }); + }); + }); +}); + +/** + * The arrow only exists in ios mode. `calculateArrowPosition` branches per side + * and every branch now runs on zoom-normalized dimensions, so each side needs + * its own coverage. + */ +configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ title, config }) => { + test.describe(title('popover: zoom'), () => { + test.beforeEach(() => { + test.info().annotations.push({ + type: 'issue', + description: 'https://github.com/ionic-team/ionic-framework/issues/30919', + }); + }); + + /** + * On the vertical sides the arrow sits between the trigger and the content, + * centered horizontally on the trigger and flush against the content edge + * it points away from. That second relationship is what `arrowHeight` + * feeds into, so it needs asserting as well as the centering. + */ + for (const side of ['top', 'bottom']) { + test(`should place the arrow between the trigger and the content when side is ${side}`, async ({ page }) => { + await page.setContent(zoomedSidePage(side, VERTICAL_SIDE_ZOOM), config); + await openPopover(page, 'trigger'); + + const triggerBox = (await page.locator('#trigger').boundingBox())!; + const contentBox = (await page.locator('ion-popover').locator('.popover-content').boundingBox())!; + const arrowBox = (await page.locator('ion-popover').locator('.popover-arrow').boundingBox())!; + + expectAligned(arrowBox.x + arrowBox.width / 2, triggerBox.x + triggerBox.width / 2); + + if (side === 'bottom') { + expectAligned(arrowBox.y + arrowBox.height, contentBox.y); + } else { + expectAligned(arrowBox.y, contentBox.y + contentBox.height); + } + }); + } + + /** + * On the horizontal sides the arrow is rotated to point sideways and is + * centered vertically on the trigger instead. + */ + for (const side of ['left', 'right']) { + test(`should center the arrow on the trigger when side is ${side}`, async ({ page }) => { + await page.setContent(zoomedSidePage(side, HORIZONTAL_SIDE_ZOOM), config); + await openPopover(page, 'trigger'); + + const triggerBox = (await page.locator('#trigger').boundingBox())!; + const arrowBox = (await page.locator('ion-popover').locator('.popover-arrow').boundingBox())!; + + expectAligned(arrowBox.y + arrowBox.height / 2, triggerBox.y + triggerBox.height / 2); + }); + } + }); +}); diff --git a/core/src/components/popover/utils.ts b/core/src/components/popover/utils.ts index 0d11a4dfeef..5c52d7d9216 100644 --- a/core/src/components/popover/utils.ts +++ b/core/src/components/popover/utils.ts @@ -105,18 +105,74 @@ export const getSafeAreaInsets = (doc: Document): SafeAreaInsets => { return insets; }; +/** + * Largest difference from 1 that the `offsetWidth` based zoom detection below + * attributes to integer rounding rather than to an actual CSS `zoom`. The + * rounding error is at most half a pixel over the width of the popover, which + * is well under this threshold for any realistic popover size. + */ +const ZOOM_ROUNDING_TOLERANCE = 0.01; + +/** + * Returns the cumulative CSS `zoom` factor applied to an element. + * + * When a CSS `zoom` other than 1 is set on an ancestor (e.g. the `html` + * element, as recommended by the docs for dynamic font scaling on Chrome for + * Android), `getBoundingClientRect()`, `clientX`/`clientY` and other geometry + * APIs report values in the *zoomed* (visual) coordinate space, while inline + * `top`/`left`/`--width` styles we set are interpreted in the *unzoomed* + * (layout) space and re-scaled by the browser. Dividing the rect-derived + * values by this factor converts them back to layout space so the popover is + * positioned and sized correctly. Returns 1 when no zoom is applied. + */ +export const getElementCSSZoom = (el: HTMLElement | null): number => { + if (!el) { + return 1; + } + + /** + * `currentCSSZoom` exposes the exact effective zoom of an element + * (Chromium 128+). When available we use it directly. + */ + const currentCSSZoom = el.currentCSSZoom; + if (typeof currentCSSZoom === 'number' && currentCSSZoom > 0) { + return currentCSSZoom; + } + + /** + * Fallback for browsers without `currentCSSZoom`: compare the rendered + * (zoomed) width from `getBoundingClientRect()` against the layout width + * from `offsetWidth`, which is not affected by CSS `zoom`. + */ + const { width } = el.getBoundingClientRect(); + const { offsetWidth } = el; + if (offsetWidth > 0 && width > 0) { + const ratio = width / offsetWidth; + /** + * `offsetWidth` is rounded to an integer while the bounding rect is not, + * so the ratio is rarely exactly 1 even when no zoom is applied. Treat + * sub-pixel differences as "no zoom" so that unzoomed popovers are not + * shifted by the rounding error. A real zoom deviates far more, though + * the same rounding leaves the detected factor approximate. + */ + return Math.abs(ratio - 1) < ZOOM_ROUNDING_TOLERANCE ? 1 : ratio; + } + + return 1; +}; + /** * Returns the dimensions of the popover * arrow on `ios` mode. If arrow is disabled * returns (0, 0). */ -export const getArrowDimensions = (arrowEl: HTMLElement | null) => { +export const getArrowDimensions = (arrowEl: HTMLElement | null, zoom = 1) => { if (!arrowEl) { return { arrowWidth: 0, arrowHeight: 0 }; } const { width, height } = arrowEl.getBoundingClientRect(); - return { arrowWidth: width, arrowHeight: height }; + return { arrowWidth: width / zoom, arrowHeight: height / zoom }; }; /** @@ -124,14 +180,14 @@ export const getArrowDimensions = (arrowEl: HTMLElement | null) => { * that takes into account whether or not the width * should match the trigger width. */ -export const getPopoverDimensions = (size: PopoverSize, contentEl: HTMLElement, triggerEl?: HTMLElement) => { +export const getPopoverDimensions = (size: PopoverSize, contentEl: HTMLElement, triggerEl?: HTMLElement, zoom = 1) => { const contentDimentions = contentEl.getBoundingClientRect(); - const contentHeight = contentDimentions.height; - let contentWidth = contentDimentions.width; + const contentHeight = contentDimentions.height / zoom; + let contentWidth = contentDimentions.width / zoom; if (size === 'cover' && triggerEl) { const triggerDimensions = triggerEl.getBoundingClientRect(); - contentWidth = triggerDimensions.width; + contentWidth = triggerDimensions.width / zoom; } return { @@ -526,7 +582,8 @@ export const getPopoverPosition = ( align: PositionAlign, defaultPosition: PopoverPosition, triggerEl?: HTMLElement, - event?: MouseEvent | CustomEvent + event?: MouseEvent | CustomEvent, + zoom = 1 ): PopoverPosition => { let referenceCoordinates = { top: 0, @@ -549,8 +606,8 @@ export const getPopoverPosition = ( const mouseEv = event as MouseEvent; referenceCoordinates = { - top: mouseEv.clientY, - left: mouseEv.clientX, + top: mouseEv.clientY / zoom, + left: mouseEv.clientX / zoom, width: 1, height: 1, }; @@ -585,10 +642,10 @@ export const getPopoverPosition = ( } const triggerBoundingBox = actualTriggerEl.getBoundingClientRect(); referenceCoordinates = { - top: triggerBoundingBox.top, - left: triggerBoundingBox.left, - width: triggerBoundingBox.width, - height: triggerBoundingBox.height, + top: triggerBoundingBox.top / zoom, + left: triggerBoundingBox.left / zoom, + width: triggerBoundingBox.width / zoom, + height: triggerBoundingBox.height / zoom, }; break; diff --git a/core/src/components/select/select.native.scss b/core/src/components/select/select.native.scss index a22aa5788b5..ed17e6bee85 100644 --- a/core/src/components/select/select.native.scss +++ b/core/src/components/select/select.native.scss @@ -74,7 +74,7 @@ // -------------------------------------------------- .select-text { - min-width: 16px; + min-width: $form-control-min-width; } // Select Label diff --git a/core/src/components/select/select.tsx b/core/src/components/select/select.tsx index 37d3db3769e..a295a538723 100644 --- a/core/src/components/select/select.tsx +++ b/core/src/components/select/select.tsx @@ -1,11 +1,26 @@ import type { ComponentInterface, EventEmitter } from '@stencil/core'; -import { Build, Component, Element, Event, Host, Method, Prop, State, Watch, h, forceUpdate } from '@stencil/core'; +import { + Build, + Component, + Element, + Event, + Host, + Listen, + Method, + Prop, + State, + Watch, + h, + forceUpdate, +} from '@stencil/core'; import { ENABLE_HTML_CONTENT_DEFAULT } from '@utils/config'; -import type { NotchController, StartContainerController } from '@utils/forms'; +import type { ClickController, NotchController, StartContainerController } from '@utils/forms'; import { compareOptions, + createClickController, createNotchController, createStartContainerController, + getSlottedClickContent, isOptionSelected, checkInvalidState, } from '@utils/forms'; @@ -93,6 +108,8 @@ export class Select implements ComponentInterface { private startContainerEl: HTMLElement | undefined; private customHTMLEnabled = config.get('innerHTMLTemplatesEnabled', ENABLE_HTML_CONTENT_DEFAULT); + private clickController?: ClickController; + @Element() el!: HTMLIonSelectElement; @State() isExpanded = false; @@ -387,6 +404,8 @@ export class Select implements ComponentInterface { this.startContainerController.calculateStartContainerWidth(); + this.clickController = createClickController(el); + this.updateOverlayOptions(); this.emitStyle(); @@ -1035,42 +1054,39 @@ export class Select implements ComponentInterface { this.ionStyle.emit(style); } + /** + * The label wrapping the slots has no `for` attribute, so the browser + * forwards a click on slotted content to the label's first labelable + * descendant, the internal button. That forwarded click bubbles back out of + * the shadow root targeting the host, where it would be emitted a second + * time and open the select. The controller swallows it during the capture + * phase, leaving the click on the slotted content itself alone so slotted + * links, checkboxes and buttons keep their default behavior. + */ + @Listen('click', { capture: true }) + onClickCapture(ev: Event) { + this.clickController?.handleClickCapture(ev); + } + private onClick = (ev: UIEvent) => { - const target = ev.target as HTMLElement; - const closestSlot = target.closest('[slot="start"], [slot="end"]'); + const slotted = getSlottedClickContent(ev, this.el); - if (target === this.el || closestSlot === null) { - this.setFocus(); - this.open(ev); - } else { - /** - * Prevent clicks to the start/end slots from opening the select. - * We ensure the target isn't this element in case the select is slotted - * in, for example, an item. This would prevent the select from ever - * being opened since the element itself has slot="start"/"end". - * - * Clicking a slotted element also causes a click - * on the