mirror of
https://github.com/ionic-team/ionic-framework.git
synced 2026-03-13 10:22:08 +08:00
fix(accordion-group): skip initial animation (#30729)
Issue number: resolves #30613 --------- <!-- 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. --> Currently, when you load an accordion group, the initially selected accordion animates open. This is an unexpected change caused by upgrading Stencil to >= 4.21.0, which changed the way component lifecycles got triggered. ## What is the new behavior? <!-- Please describe the behavior or changes that are being added by this PR. --> With this change, we're waiting for the accordion in the accordion group to render and telling it if the update it's going through is the initial update or not. This allows it to decide to animate properly. ## 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. --> Current dev build: ``` 8.7.8-dev.11761840817.1bede576 ```
This commit is contained in:
@@ -38,7 +38,40 @@ const enum AccordionState {
|
||||
})
|
||||
export class Accordion implements ComponentInterface {
|
||||
private accordionGroupEl?: HTMLIonAccordionGroupElement | null;
|
||||
private updateListener = () => this.updateState(false);
|
||||
private accordionGroupUpdateHandler = () => {
|
||||
/**
|
||||
* Determine if this update will cause an actual state change.
|
||||
* We only want to mark as "interacted" if the state is changing.
|
||||
*/
|
||||
const accordionGroup = this.accordionGroupEl;
|
||||
if (accordionGroup) {
|
||||
const value = accordionGroup.value;
|
||||
const accordionValue = this.value;
|
||||
const shouldExpand = Array.isArray(value) ? value.includes(accordionValue) : value === accordionValue;
|
||||
const isExpanded = this.state === AccordionState.Expanded || this.state === AccordionState.Expanding;
|
||||
const stateWillChange = shouldExpand !== isExpanded;
|
||||
|
||||
/**
|
||||
* Only mark as interacted if:
|
||||
* 1. This is not the first update we've received with a defined value
|
||||
* 2. The state is actually changing (prevents redundant updates from enabling animations)
|
||||
*/
|
||||
if (this.hasReceivedFirstUpdate && stateWillChange) {
|
||||
this.hasInteracted = true;
|
||||
}
|
||||
|
||||
/**
|
||||
* Only count this as the first update if the group value is defined.
|
||||
* This prevents the initial undefined value from the group's componentDidLoad
|
||||
* from being treated as the first real update.
|
||||
*/
|
||||
if (value !== undefined) {
|
||||
this.hasReceivedFirstUpdate = true;
|
||||
}
|
||||
}
|
||||
|
||||
this.updateState();
|
||||
};
|
||||
private contentEl: HTMLDivElement | undefined;
|
||||
private contentElWrapper: HTMLDivElement | undefined;
|
||||
private headerEl: HTMLDivElement | undefined;
|
||||
@@ -50,6 +83,25 @@ export class Accordion implements ComponentInterface {
|
||||
@State() state: AccordionState = AccordionState.Collapsed;
|
||||
@State() isNext = false;
|
||||
@State() isPrevious = false;
|
||||
/**
|
||||
* Tracks whether a user-initiated interaction has occurred.
|
||||
* Animations are disabled until the first interaction happens.
|
||||
* This prevents the accordion from animating when it's programmatically
|
||||
* set to an expanded or collapsed state on initial load.
|
||||
*/
|
||||
@State() hasInteracted = false;
|
||||
|
||||
/**
|
||||
* Tracks if this accordion has ever been expanded.
|
||||
* Used to prevent the first expansion from animating.
|
||||
*/
|
||||
private hasEverBeenExpanded = false;
|
||||
|
||||
/**
|
||||
* Tracks if this accordion has received its first update from the group.
|
||||
* Used to distinguish initial programmatic sets from user interactions.
|
||||
*/
|
||||
private hasReceivedFirstUpdate = false;
|
||||
|
||||
/**
|
||||
* The value of the accordion. Defaults to an autogenerated
|
||||
@@ -88,15 +140,15 @@ export class Accordion implements ComponentInterface {
|
||||
connectedCallback() {
|
||||
const accordionGroupEl = (this.accordionGroupEl = this.el?.closest('ion-accordion-group'));
|
||||
if (accordionGroupEl) {
|
||||
this.updateState(true);
|
||||
addEventListener(accordionGroupEl, 'ionValueChange', this.updateListener);
|
||||
this.updateState();
|
||||
addEventListener(accordionGroupEl, 'ionValueChange', this.accordionGroupUpdateHandler);
|
||||
}
|
||||
}
|
||||
|
||||
disconnectedCallback() {
|
||||
const accordionGroupEl = this.accordionGroupEl;
|
||||
if (accordionGroupEl) {
|
||||
removeEventListener(accordionGroupEl, 'ionValueChange', this.updateListener);
|
||||
removeEventListener(accordionGroupEl, 'ionValueChange', this.accordionGroupUpdateHandler);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -212,10 +264,16 @@ export class Accordion implements ComponentInterface {
|
||||
ionItem.appendChild(iconEl);
|
||||
};
|
||||
|
||||
private expandAccordion = (initialUpdate = false) => {
|
||||
private expandAccordion = () => {
|
||||
const { contentEl, contentElWrapper } = this;
|
||||
if (initialUpdate || contentEl === undefined || contentElWrapper === undefined) {
|
||||
|
||||
/**
|
||||
* If the content elements aren't available yet, just set the state.
|
||||
* This happens on initial render before the DOM is ready.
|
||||
*/
|
||||
if (contentEl === undefined || contentElWrapper === undefined) {
|
||||
this.state = AccordionState.Expanded;
|
||||
this.hasEverBeenExpanded = true;
|
||||
return;
|
||||
}
|
||||
|
||||
@@ -227,6 +285,12 @@ export class Accordion implements ComponentInterface {
|
||||
cancelAnimationFrame(this.currentRaf);
|
||||
}
|
||||
|
||||
/**
|
||||
* Mark that this accordion has been expanded at least once.
|
||||
* This allows subsequent expansions to animate.
|
||||
*/
|
||||
this.hasEverBeenExpanded = true;
|
||||
|
||||
if (this.shouldAnimate()) {
|
||||
raf(() => {
|
||||
this.state = AccordionState.Expanding;
|
||||
@@ -247,9 +311,14 @@ export class Accordion implements ComponentInterface {
|
||||
}
|
||||
};
|
||||
|
||||
private collapseAccordion = (initialUpdate = false) => {
|
||||
private collapseAccordion = () => {
|
||||
const { contentEl } = this;
|
||||
if (initialUpdate || contentEl === undefined) {
|
||||
|
||||
/**
|
||||
* If the content element isn't available yet, just set the state.
|
||||
* This happens on initial render before the DOM is ready.
|
||||
*/
|
||||
if (contentEl === undefined) {
|
||||
this.state = AccordionState.Collapsed;
|
||||
return;
|
||||
}
|
||||
@@ -291,6 +360,19 @@ export class Accordion implements ComponentInterface {
|
||||
* of what is set in the config.
|
||||
*/
|
||||
private shouldAnimate = () => {
|
||||
/**
|
||||
* Don't animate until after the first user interaction.
|
||||
* This prevents animations on initial load when accordions
|
||||
* start in an expanded or collapsed state programmatically.
|
||||
*
|
||||
* Additionally, don't animate the very first expansion even if
|
||||
* hasInteracted is true. This handles edge cases like React StrictMode
|
||||
* where effects run twice and might incorrectly mark as interacted.
|
||||
*/
|
||||
if (!this.hasInteracted || !this.hasEverBeenExpanded) {
|
||||
return false;
|
||||
}
|
||||
|
||||
if (typeof (window as any) === 'undefined') {
|
||||
return false;
|
||||
}
|
||||
@@ -312,7 +394,7 @@ export class Accordion implements ComponentInterface {
|
||||
return true;
|
||||
};
|
||||
|
||||
private updateState = async (initialUpdate = false) => {
|
||||
private updateState = async () => {
|
||||
const accordionGroup = this.accordionGroupEl;
|
||||
const accordionValue = this.value;
|
||||
|
||||
@@ -325,10 +407,10 @@ export class Accordion implements ComponentInterface {
|
||||
const shouldExpand = Array.isArray(value) ? value.includes(accordionValue) : value === accordionValue;
|
||||
|
||||
if (shouldExpand) {
|
||||
this.expandAccordion(initialUpdate);
|
||||
this.expandAccordion();
|
||||
this.isNext = this.isPrevious = false;
|
||||
} else {
|
||||
this.collapseAccordion(initialUpdate);
|
||||
this.collapseAccordion();
|
||||
|
||||
/**
|
||||
* When using popout or inset,
|
||||
@@ -386,6 +468,12 @@ export class Accordion implements ComponentInterface {
|
||||
|
||||
if (disabled || readonly) return;
|
||||
|
||||
/**
|
||||
* Mark that the user has interacted with the accordion.
|
||||
* This enables animations for all future state changes.
|
||||
*/
|
||||
this.hasInteracted = true;
|
||||
|
||||
if (accordionGroupEl) {
|
||||
/**
|
||||
* Because the accordion group may or may
|
||||
|
||||
@@ -200,6 +200,87 @@ it('should set default values if not provided', async () => {
|
||||
expect(accordion.classList.contains('accordion-collapsed')).toEqual(false);
|
||||
});
|
||||
|
||||
it('should not animate when initial value is set before load', async () => {
|
||||
const page = await newSpecPage({
|
||||
components: [Item, Accordion, AccordionGroup],
|
||||
});
|
||||
|
||||
const accordionGroup = page.doc.createElement('ion-accordion-group');
|
||||
accordionGroup.innerHTML = `
|
||||
<ion-accordion value="first">
|
||||
<ion-item slot="header">Label</ion-item>
|
||||
<div slot="content">Content</div>
|
||||
</ion-accordion>
|
||||
<ion-accordion value="second">
|
||||
<ion-item slot="header">Label</ion-item>
|
||||
<div slot="content">Content</div>
|
||||
</ion-accordion>
|
||||
`;
|
||||
|
||||
accordionGroup.value = 'first';
|
||||
page.body.appendChild(accordionGroup);
|
||||
|
||||
await page.waitForChanges();
|
||||
|
||||
const firstAccordion = accordionGroup.querySelector('ion-accordion[value="first"]')!;
|
||||
|
||||
expect(firstAccordion.classList.contains('accordion-expanded')).toEqual(true);
|
||||
expect(firstAccordion.classList.contains('accordion-expanding')).toEqual(false);
|
||||
});
|
||||
|
||||
it('should not animate when initial value is set after load', async () => {
|
||||
const page = await newSpecPage({
|
||||
components: [Item, Accordion, AccordionGroup],
|
||||
});
|
||||
|
||||
const accordionGroup = page.doc.createElement('ion-accordion-group');
|
||||
accordionGroup.innerHTML = `
|
||||
<ion-accordion value="first">
|
||||
<ion-item slot="header">Label</ion-item>
|
||||
<div slot="content">Content</div>
|
||||
</ion-accordion>
|
||||
<ion-accordion value="second">
|
||||
<ion-item slot="header">Label</ion-item>
|
||||
<div slot="content">Content</div>
|
||||
</ion-accordion>
|
||||
`;
|
||||
|
||||
page.body.appendChild(accordionGroup);
|
||||
await page.waitForChanges();
|
||||
|
||||
accordionGroup.value = 'first';
|
||||
await page.waitForChanges();
|
||||
|
||||
const firstAccordion = accordionGroup.querySelector('ion-accordion[value="first"]')!;
|
||||
|
||||
expect(firstAccordion.classList.contains('accordion-expanded')).toEqual(true);
|
||||
expect(firstAccordion.classList.contains('accordion-expanding')).toEqual(false);
|
||||
});
|
||||
|
||||
it('should not have animated class on first expansion', async () => {
|
||||
const page = await newSpecPage({
|
||||
components: [Item, Accordion, AccordionGroup],
|
||||
html: `
|
||||
<ion-accordion-group>
|
||||
<ion-accordion value="first">
|
||||
<ion-item slot="header">Label</ion-item>
|
||||
<div slot="content">Content</div>
|
||||
</ion-accordion>
|
||||
</ion-accordion-group>
|
||||
`,
|
||||
});
|
||||
|
||||
const accordionGroup = page.body.querySelector('ion-accordion-group')!;
|
||||
const firstAccordion = page.body.querySelector('ion-accordion[value="first"]')!;
|
||||
|
||||
// First expansion should not have the animated class
|
||||
accordionGroup.value = 'first';
|
||||
await page.waitForChanges();
|
||||
|
||||
expect(firstAccordion.classList.contains('accordion-animated')).toEqual(false);
|
||||
expect(firstAccordion.classList.contains('accordion-expanded')).toEqual(true);
|
||||
});
|
||||
|
||||
// Verifies fix for https://github.com/ionic-team/ionic-framework/issues/27047
|
||||
it('should not have animated class when animated="false"', async () => {
|
||||
const page = await newSpecPage({
|
||||
|
||||
@@ -37,6 +37,7 @@ import KeepContentsMounted from './pages/overlay-components/KeepContentsMounted'
|
||||
import OverlayComponents from './pages/overlay-components/OverlayComponents';
|
||||
import OverlayHooks from './pages/overlay-hooks/OverlayHooks';
|
||||
import ReorderGroup from './pages/ReorderGroup';
|
||||
import AccordionGroup from './pages/AccordionGroup';
|
||||
|
||||
setupIonicReact();
|
||||
|
||||
@@ -69,6 +70,7 @@ const App: React.FC = () => (
|
||||
<Route path="/icons" component={Icons} />
|
||||
<Route path="/inputs" component={Inputs} />
|
||||
<Route path="/reorder-group" component={ReorderGroup} />
|
||||
<Route path="/accordion-group" component={AccordionGroup} />
|
||||
</IonRouterOutlet>
|
||||
</IonReactRouter>
|
||||
</IonApp>
|
||||
|
||||
54
packages/react/test/base/src/pages/AccordionGroup.tsx
Normal file
54
packages/react/test/base/src/pages/AccordionGroup.tsx
Normal file
@@ -0,0 +1,54 @@
|
||||
import { IonHeader, IonTitle, IonToolbar, IonPage, IonContent, IonAccordionGroup, IonAccordion, IonItem, IonLabel } from '@ionic/react';
|
||||
import { useEffect, useRef } from 'react';
|
||||
|
||||
const AccordionGroup: React.FC = () => {
|
||||
const accordionGroup = useRef<null | HTMLIonAccordionGroupElement>(null);
|
||||
|
||||
useEffect(() => {
|
||||
if (!accordionGroup.current) {
|
||||
return;
|
||||
}
|
||||
|
||||
accordionGroup.current.value = ['first', 'third'];
|
||||
}, []);
|
||||
|
||||
return (
|
||||
<IonPage>
|
||||
<IonHeader>
|
||||
<IonToolbar>
|
||||
<IonTitle>Accordion Group</IonTitle>
|
||||
</IonToolbar>
|
||||
</IonHeader>
|
||||
<IonContent>
|
||||
<IonAccordionGroup ref={accordionGroup} multiple={true}>
|
||||
<IonAccordion value="first">
|
||||
<IonItem slot="header" color="light">
|
||||
<IonLabel>First Accordion</IonLabel>
|
||||
</IonItem>
|
||||
<div className="ion-padding" slot="content">
|
||||
First Content
|
||||
</div>
|
||||
</IonAccordion>
|
||||
<IonAccordion value="second">
|
||||
<IonItem slot="header" color="light">
|
||||
<IonLabel>Second Accordion</IonLabel>
|
||||
</IonItem>
|
||||
<div className="ion-padding" slot="content">
|
||||
Second Content
|
||||
</div>
|
||||
</IonAccordion>
|
||||
<IonAccordion value="third">
|
||||
<IonItem slot="header" color="light">
|
||||
<IonLabel>Third Accordion</IonLabel>
|
||||
</IonItem>
|
||||
<div className="ion-padding" slot="content">
|
||||
Third Content
|
||||
</div>
|
||||
</IonAccordion>
|
||||
</IonAccordionGroup>
|
||||
</IonContent>
|
||||
</IonPage>
|
||||
);
|
||||
};
|
||||
|
||||
export default AccordionGroup;
|
||||
@@ -22,6 +22,9 @@ const Main: React.FC<MainProps> = () => {
|
||||
</IonHeader>
|
||||
<IonContent>
|
||||
<IonList>
|
||||
<IonItem routerLink="/accordion-group">
|
||||
<IonLabel>Accordion Group</IonLabel>
|
||||
</IonItem>
|
||||
<IonItem routerLink="/overlay-hooks">
|
||||
<IonLabel>Overlay Hooks</IonLabel>
|
||||
</IonItem>
|
||||
|
||||
Reference in New Issue
Block a user