Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

Commit 58d5638

Browse files
authored
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 ```
1 parent cfd8c42 commit 58d5638

File tree

5 files changed

+239
-11
lines changed

5 files changed

+239
-11
lines changed

‎core/src/components/accordion/accordion.tsx‎

Lines changed: 99 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,40 @@ const enum AccordionState {
3838
})
3939
export class Accordion implements ComponentInterface {
4040
private accordionGroupEl?: HTMLIonAccordionGroupElement | null;
41-
private updateListener = () => this.updateState(false);
41+
private accordionGroupUpdateHandler = () => {
42+
/**
43+
* Determine if this update will cause an actual state change.
44+
* We only want to mark as "interacted" if the state is changing.
45+
*/
46+
const accordionGroup = this.accordionGroupEl;
47+
if (accordionGroup) {
48+
const value = accordionGroup.value;
49+
const accordionValue = this.value;
50+
const shouldExpand = Array.isArray(value) ? value.includes(accordionValue) : value === accordionValue;
51+
const isExpanded = this.state === AccordionState.Expanded || this.state === AccordionState.Expanding;
52+
const stateWillChange = shouldExpand !== isExpanded;
53+
54+
/**
55+
* Only mark as interacted if:
56+
* 1. This is not the first update we've received with a defined value
57+
* 2. The state is actually changing (prevents redundant updates from enabling animations)
58+
*/
59+
if (this.hasReceivedFirstUpdate && stateWillChange) {
60+
this.hasInteracted = true;
61+
}
62+
63+
/**
64+
* Only count this as the first update if the group value is defined.
65+
* This prevents the initial undefined value from the group's componentDidLoad
66+
* from being treated as the first real update.
67+
*/
68+
if (value !== undefined) {
69+
this.hasReceivedFirstUpdate = true;
70+
}
71+
}
72+
73+
this.updateState();
74+
};
4275
private contentEl: HTMLDivElement | undefined;
4376
private contentElWrapper: HTMLDivElement | undefined;
4477
private headerEl: HTMLDivElement | undefined;
@@ -50,6 +83,25 @@ export class Accordion implements ComponentInterface {
5083
@State() state: AccordionState = AccordionState.Collapsed;
5184
@State() isNext = false;
5285
@State() isPrevious = false;
86+
/**
87+
* Tracks whether a user-initiated interaction has occurred.
88+
* Animations are disabled until the first interaction happens.
89+
* This prevents the accordion from animating when it's programmatically
90+
* set to an expanded or collapsed state on initial load.
91+
*/
92+
@State() hasInteracted = false;
93+
94+
/**
95+
* Tracks if this accordion has ever been expanded.
96+
* Used to prevent the first expansion from animating.
97+
*/
98+
private hasEverBeenExpanded = false;
99+
100+
/**
101+
* Tracks if this accordion has received its first update from the group.
102+
* Used to distinguish initial programmatic sets from user interactions.
103+
*/
104+
private hasReceivedFirstUpdate = false;
53105

54106
/**
55107
* The value of the accordion. Defaults to an autogenerated
@@ -88,15 +140,15 @@ export class Accordion implements ComponentInterface {
88140
connectedCallback() {
89141
const accordionGroupEl = (this.accordionGroupEl = this.el?.closest('ion-accordion-group'));
90142
if (accordionGroupEl) {
91-
this.updateState(true);
92-
addEventListener(accordionGroupEl, 'ionValueChange', this.updateListener);
143+
this.updateState();
144+
addEventListener(accordionGroupEl, 'ionValueChange', this.accordionGroupUpdateHandler);
93145
}
94146
}
95147

96148
disconnectedCallback() {
97149
const accordionGroupEl = this.accordionGroupEl;
98150
if (accordionGroupEl) {
99-
removeEventListener(accordionGroupEl, 'ionValueChange', this.updateListener);
151+
removeEventListener(accordionGroupEl, 'ionValueChange', this.accordionGroupUpdateHandler);
100152
}
101153
}
102154

@@ -212,10 +264,16 @@ export class Accordion implements ComponentInterface {
212264
ionItem.appendChild(iconEl);
213265
};
214266

215-
private expandAccordion = (initialUpdate=false) => {
267+
private expandAccordion = () => {
216268
const { contentEl, contentElWrapper } = this;
217-
if (initialUpdate || contentEl === undefined || contentElWrapper === undefined) {
269+
270+
/**
271+
* If the content elements aren't available yet, just set the state.
272+
* This happens on initial render before the DOM is ready.
273+
*/
274+
if (contentEl === undefined || contentElWrapper === undefined) {
218275
this.state = AccordionState.Expanded;
276+
this.hasEverBeenExpanded = true;
219277
return;
220278
}
221279

@@ -227,6 +285,12 @@ export class Accordion implements ComponentInterface {
227285
cancelAnimationFrame(this.currentRaf);
228286
}
229287

288+
/**
289+
* Mark that this accordion has been expanded at least once.
290+
* This allows subsequent expansions to animate.
291+
*/
292+
this.hasEverBeenExpanded = true;
293+
230294
if (this.shouldAnimate()) {
231295
raf(() => {
232296
this.state = AccordionState.Expanding;
@@ -247,9 +311,14 @@ export class Accordion implements ComponentInterface {
247311
}
248312
};
249313

250-
private collapseAccordion = (initialUpdate=false) => {
314+
private collapseAccordion = () => {
251315
const { contentEl } = this;
252-
if (initialUpdate || contentEl === undefined) {
316+
317+
/**
318+
* If the content element isn't available yet, just set the state.
319+
* This happens on initial render before the DOM is ready.
320+
*/
321+
if (contentEl === undefined) {
253322
this.state = AccordionState.Collapsed;
254323
return;
255324
}
@@ -291,6 +360,19 @@ export class Accordion implements ComponentInterface {
291360
* of what is set in the config.
292361
*/
293362
private shouldAnimate = () => {
363+
/**
364+
* Don't animate until after the first user interaction.
365+
* This prevents animations on initial load when accordions
366+
* start in an expanded or collapsed state programmatically.
367+
*
368+
* Additionally, don't animate the very first expansion even if
369+
* hasInteracted is true. This handles edge cases like React StrictMode
370+
* where effects run twice and might incorrectly mark as interacted.
371+
*/
372+
if (!this.hasInteracted || !this.hasEverBeenExpanded) {
373+
return false;
374+
}
375+
294376
if (typeof (window as any) === 'undefined') {
295377
return false;
296378
}
@@ -312,7 +394,7 @@ export class Accordion implements ComponentInterface {
312394
return true;
313395
};
314396

315-
private updateState = async (initialUpdate=false) => {
397+
private updateState = async () => {
316398
const accordionGroup = this.accordionGroupEl;
317399
const accordionValue = this.value;
318400

@@ -325,10 +407,10 @@ export class Accordion implements ComponentInterface {
325407
const shouldExpand = Array.isArray(value) ? value.includes(accordionValue) : value === accordionValue;
326408

327409
if (shouldExpand) {
328-
this.expandAccordion(initialUpdate);
410+
this.expandAccordion();
329411
this.isNext = this.isPrevious = false;
330412
} else {
331-
this.collapseAccordion(initialUpdate);
413+
this.collapseAccordion();
332414

333415
/**
334416
* When using popout or inset,
@@ -386,6 +468,12 @@ export class Accordion implements ComponentInterface {
386468

387469
if (disabled || readonly) return;
388470

471+
/**
472+
* Mark that the user has interacted with the accordion.
473+
* This enables animations for all future state changes.
474+
*/
475+
this.hasInteracted = true;
476+
389477
if (accordionGroupEl) {
390478
/**
391479
* Because the accordion group may or may

‎core/src/components/accordion/test/accordion.spec.ts‎

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -200,6 +200,87 @@ it('should set default values if not provided', async () => {
200200
expect(accordion.classList.contains('accordion-collapsed')).toEqual(false);
201201
});
202202

203+
it('should not animate when initial value is set before load', async () => {
204+
const page = await newSpecPage({
205+
components: [Item, Accordion, AccordionGroup],
206+
});
207+
208+
const accordionGroup = page.doc.createElement('ion-accordion-group');
209+
accordionGroup.innerHTML = `
210+
<ion-accordion value="first">
211+
<ion-item slot="header">Label</ion-item>
212+
<div slot="content">Content</div>
213+
</ion-accordion>
214+
<ion-accordion value="second">
215+
<ion-item slot="header">Label</ion-item>
216+
<div slot="content">Content</div>
217+
</ion-accordion>
218+
`;
219+
220+
accordionGroup.value = 'first';
221+
page.body.appendChild(accordionGroup);
222+
223+
await page.waitForChanges();
224+
225+
const firstAccordion = accordionGroup.querySelector('ion-accordion[value="first"]')!;
226+
227+
expect(firstAccordion.classList.contains('accordion-expanded')).toEqual(true);
228+
expect(firstAccordion.classList.contains('accordion-expanding')).toEqual(false);
229+
});
230+
231+
it('should not animate when initial value is set after load', async () => {
232+
const page = await newSpecPage({
233+
components: [Item, Accordion, AccordionGroup],
234+
});
235+
236+
const accordionGroup = page.doc.createElement('ion-accordion-group');
237+
accordionGroup.innerHTML = `
238+
<ion-accordion value="first">
239+
<ion-item slot="header">Label</ion-item>
240+
<div slot="content">Content</div>
241+
</ion-accordion>
242+
<ion-accordion value="second">
243+
<ion-item slot="header">Label</ion-item>
244+
<div slot="content">Content</div>
245+
</ion-accordion>
246+
`;
247+
248+
page.body.appendChild(accordionGroup);
249+
await page.waitForChanges();
250+
251+
accordionGroup.value = 'first';
252+
await page.waitForChanges();
253+
254+
const firstAccordion = accordionGroup.querySelector('ion-accordion[value="first"]')!;
255+
256+
expect(firstAccordion.classList.contains('accordion-expanded')).toEqual(true);
257+
expect(firstAccordion.classList.contains('accordion-expanding')).toEqual(false);
258+
});
259+
260+
it('should not have animated class on first expansion', async () => {
261+
const page = await newSpecPage({
262+
components: [Item, Accordion, AccordionGroup],
263+
html: `
264+
<ion-accordion-group>
265+
<ion-accordion value="first">
266+
<ion-item slot="header">Label</ion-item>
267+
<div slot="content">Content</div>
268+
</ion-accordion>
269+
</ion-accordion-group>
270+
`,
271+
});
272+
273+
const accordionGroup = page.body.querySelector('ion-accordion-group')!;
274+
const firstAccordion = page.body.querySelector('ion-accordion[value="first"]')!;
275+
276+
// First expansion should not have the animated class
277+
accordionGroup.value = 'first';
278+
await page.waitForChanges();
279+
280+
expect(firstAccordion.classList.contains('accordion-animated')).toEqual(false);
281+
expect(firstAccordion.classList.contains('accordion-expanded')).toEqual(true);
282+
});
283+
203284
// Verifies fix for https://github.com/ionic-team/ionic-framework/issues/27047
204285
it('should not have animated class when animated="false"', async () => {
205286
const page = await newSpecPage({

‎packages/react/test/base/src/App.tsx‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ import KeepContentsMounted from './pages/overlay-components/KeepContentsMounted'
3737
import OverlayComponents from './pages/overlay-components/OverlayComponents';
3838
import OverlayHooks from './pages/overlay-hooks/OverlayHooks';
3939
import ReorderGroup from './pages/ReorderGroup';
40+
import AccordionGroup from './pages/AccordionGroup';
4041

4142
setupIonicReact();
4243

@@ -69,6 +70,7 @@ const App: React.FC = () => (
6970
<Route path="/icons" component={Icons} />
7071
<Route path="/inputs" component={Inputs} />
7172
<Route path="/reorder-group" component={ReorderGroup} />
73+
<Route path="/accordion-group" component={AccordionGroup} />
7274
</IonRouterOutlet>
7375
</IonReactRouter>
7476
</IonApp>
Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
import { IonHeader, IonTitle, IonToolbar, IonPage, IonContent, IonAccordionGroup, IonAccordion, IonItem, IonLabel } from '@ionic/react';
2+
import { useEffect, useRef } from 'react';
3+
4+
const AccordionGroup: React.FC = () => {
5+
const accordionGroup = useRef<null | HTMLIonAccordionGroupElement>(null);
6+
7+
useEffect(() => {
8+
if (!accordionGroup.current) {
9+
return;
10+
}
11+
12+
accordionGroup.current.value = ['first', 'third'];
13+
}, []);
14+
15+
return (
16+
<IonPage>
17+
<IonHeader>
18+
<IonToolbar>
19+
<IonTitle>Accordion Group</IonTitle>
20+
</IonToolbar>
21+
</IonHeader>
22+
<IonContent>
23+
<IonAccordionGroup ref={accordionGroup} multiple={true}>
24+
<IonAccordion value="first">
25+
<IonItem slot="header" color="light">
26+
<IonLabel>First Accordion</IonLabel>
27+
</IonItem>
28+
<div className="ion-padding" slot="content">
29+
First Content
30+
</div>
31+
</IonAccordion>
32+
<IonAccordion value="second">
33+
<IonItem slot="header" color="light">
34+
<IonLabel>Second Accordion</IonLabel>
35+
</IonItem>
36+
<div className="ion-padding" slot="content">
37+
Second Content
38+
</div>
39+
</IonAccordion>
40+
<IonAccordion value="third">
41+
<IonItem slot="header" color="light">
42+
<IonLabel>Third Accordion</IonLabel>
43+
</IonItem>
44+
<div className="ion-padding" slot="content">
45+
Third Content
46+
</div>
47+
</IonAccordion>
48+
</IonAccordionGroup>
49+
</IonContent>
50+
</IonPage>
51+
);
52+
};
53+
54+
export default AccordionGroup;

‎packages/react/test/base/src/pages/Main.tsx‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,9 @@ const Main: React.FC<MainProps> = () => {
2222
</IonHeader>
2323
<IonContent>
2424
<IonList>
25+
<IonItem routerLink="/accordion-group">
26+
<IonLabel>Accordion Group</IonLabel>
27+
</IonItem>
2528
<IonItem routerLink="/overlay-hooks">
2629
<IonLabel>Overlay Hooks</IonLabel>
2730
</IonItem>

0 commit comments

Comments
(0)

AltStyle によって変換されたページ (->オリジナル) /