From 3f730ab1d77be54d1faf14168eee9e9dc41002d6 Mon Sep 17 00:00:00 2001 From: Colin Bares Date: Tue, 15 Jul 2025 11:17:19 -0500 Subject: [PATCH] fix(item): allow nested content to be conditionally interactive (#30519) Issue number: resolves #29763 --------- ## What is the current behavior? If the nested content of an ion-item is conditionally rendered and goes from containing zero interactive elements to one or more, a render is not triggered in Angular and the item does not behave correctly for one or more nested inputs. ## What is the new behavior? - A mutation observer is created in `connectedCallback()` to watch for changes in item's child list. When the `childList` changes, two pieces of state that track whether the item needs to be interactive and whether there are multiple interactive elements are updated. - Add `disconnectedCallback()` and logic to disconnect Mutation Observer. - Create new function `totalNestedInputs()` with logic from existing `setMultipleInputs` function to be used for both `setMultipleInputs` and new function `setIsInteractive`. ## Does this introduce a breaking change? - [ ] Yes - [x] No ## Other information I opted for the MutationObserver over a `slotchange` listener because the `slotchange` fires synchronously on any slot within the shadowRoot, and the MutationObserver fires once on the item element itself. I attempted to add the minimum amount of logic to achieve this but there may be a more elegant solution that combines what `multipleInputs` and `isInteractive` are doing but requires more changes to existing code. --- core/src/components/item/item.tsx | 39 ++++++++++++++---- .../components/item/test/inputs/item.e2e.ts | 41 +++++++++++++++++++ 2 files changed, 73 insertions(+), 7 deletions(-) diff --git a/core/src/components/item/item.tsx b/core/src/components/item/item.tsx index 6384927de8..e5269bb4f0 100644 --- a/core/src/components/item/item.tsx +++ b/core/src/components/item/item.tsx @@ -37,6 +37,7 @@ export class Item implements ComponentInterface, AnchorInterface, ButtonInterfac @State() multipleInputs = false; @State() focusable = true; + @State() isInteractive = false; /** * The color to use from your application's color palette. @@ -172,14 +173,12 @@ export class Item implements ComponentInterface, AnchorInterface, ButtonInterfac componentDidLoad() { raf(() => { this.setMultipleInputs(); + this.setIsInteractive(); this.focusable = this.isFocusable(); }); } - // If the item contains multiple clickable elements and/or inputs, then the item - // should not have a clickable input cover over the entire item to prevent - // interfering with their individual click events - private setMultipleInputs() { + private totalNestedInputs() { // The following elements have a clickable cover that is relative to the entire item const covers = this.el.querySelectorAll('ion-checkbox, ion-datetime, ion-select, ion-radio'); @@ -193,6 +192,19 @@ export class Item implements ComponentInterface, AnchorInterface, ButtonInterfac // The following elements should also stay clickable when an input with cover is present const clickables = this.el.querySelectorAll('ion-router-link, ion-button, a, button'); + return { + covers, + inputs, + clickables, + }; + } + + // If the item contains multiple clickable elements and/or inputs, then the item + // should not have a clickable input cover over the entire item to prevent + // interfering with their individual click events + private setMultipleInputs() { + const { covers, inputs, clickables } = this.totalNestedInputs(); + // Check for multiple inputs to change the position of the input cover to relative // for all of the covered inputs above this.multipleInputs = @@ -201,6 +213,19 @@ export class Item implements ComponentInterface, AnchorInterface, ButtonInterfac (covers.length > 0 && this.isClickable()); } + private setIsInteractive() { + // If item contains any interactive children, set isInteractive to `true` + const { covers, inputs, clickables } = this.totalNestedInputs(); + + this.isInteractive = covers.length > 0 || inputs.length > 0 || clickables.length > 0; + } + + // slot change listener updates state to reflect how/if item should be interactive + private updateInteractivityOnSlotChange = () => { + this.setIsInteractive(); + this.setMultipleInputs(); + }; + // If the item contains an input including a checkbox, datetime, select, or radio // then the item will have a clickable input cover that covers the item // that should get the hover, focused and activated states UNLESS it has multiple @@ -364,12 +389,12 @@ export class Item implements ComponentInterface, AnchorInterface, ButtonInterfac disabled={disabled} {...clickFn} > - +
- +
- + {showDetail && ( { await expect(list).toHaveScreenshot(screenshot(`item-inputs-div-with-inputs`)); }); + + test('should update interactivity state when elements are conditionally rendered', async ({ page }, testInfo) => { + testInfo.annotations.push({ + type: 'issue', + description: 'https://github.com/ionic-team/ionic-framework/issues/29763', + }); + + await page.setContent( + ` + + + Conditional Checkbox + + + `, + config + ); + + const item = page.locator('ion-item'); + + await page.evaluate(() => { + const item = document.querySelector('ion-item'); + const checkbox = document.createElement('ion-checkbox'); + item?.appendChild(checkbox); + }); + + await page.waitForChanges(); + + const checkbox = page.locator('ion-checkbox'); + await expect(checkbox).not.toBeChecked(); + + // Test that clicking on the left edge of the item toggles the checkbox + await item.click({ + position: { + x: 5, + y: 5, + }, + }); + + await expect(checkbox).toBeChecked(); + }); }); });