mirror of
https://github.com/ionic-team/ionic-framework.git
synced 2025-11-08 23:58:13 +08:00
fix(item): allow nested content to be conditionally interactive (#30519)
Issue number: resolves #29763 --------- <!-- Please do not submit updates to dependencies unless it fixes an issue. --> <!-- Please try to limit your pull request to one type (bugfix, feature, etc). Submit multiple pull requests if needed. --> ## What is the current behavior? <!-- Please describe the current behavior that you are modifying. --> 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? <!-- Please describe the behavior or changes that are being added by this PR. --> - 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 <!-- If this introduces a breaking change: 1. Describe the impact and migration path for existing applications below. 2. Update the BREAKING.md file with the breaking change. 3. Add "BREAKING CHANGE: [...]" to the commit description when merging. See https://github.com/ionic-team/ionic-framework/blob/main/docs/CONTRIBUTING.md#footer for more information. --> ## Other information <!-- Any other information that is important to this PR such as screenshots of how the component looks before and after the change. --> 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.
This commit is contained in:
@ -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}
|
||||
>
|
||||
<slot name="start"></slot>
|
||||
<slot name="start" onSlotchange={this.updateInteractivityOnSlotChange}></slot>
|
||||
<div class="item-inner">
|
||||
<div class="input-wrapper">
|
||||
<slot></slot>
|
||||
<slot onSlotchange={this.updateInteractivityOnSlotChange}></slot>
|
||||
</div>
|
||||
<slot name="end"></slot>
|
||||
<slot name="end" onSlotchange={this.updateInteractivityOnSlotChange}></slot>
|
||||
{showDetail && (
|
||||
<ion-icon
|
||||
icon={detailIcon}
|
||||
|
||||
@ -252,5 +252,46 @@ configs({ directions: ['ltr'] }).forEach(({ title, screenshot, config }) => {
|
||||
|
||||
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(
|
||||
`
|
||||
<ion-list>
|
||||
<ion-item>
|
||||
<ion-label>Conditional Checkbox</ion-label>
|
||||
</ion-item>
|
||||
</ion-list>
|
||||
`,
|
||||
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();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user