Skip to content

Commit 22f452f

Browse files
committed
fix(accordion): trying to prevent animation based on interaction rather than render
1 parent f84c484 commit 22f452f

File tree

1 file changed

+110
-21
lines changed

1 file changed

+110
-21
lines changed

core/src/components/accordion/accordion.tsx

Lines changed: 110 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,53 @@ export class Accordion implements ComponentInterface {
4141
private accordionGroupEl?: HTMLIonAccordionGroupElement | null;
4242
private updateListener = (ev: CustomEvent<AccordionGroupChangeEventDetail>) => {
4343
const initialUpdate = ev.detail?.initial ?? false;
44+
console.log(
45+
'[Accordion]',
46+
this.value,
47+
'updateListener - initial:',
48+
initialUpdate,
49+
'hasInteracted:',
50+
this.hasInteracted,
51+
'hasEverBeenExpanded:',
52+
this.hasEverBeenExpanded,
53+
'currentState:',
54+
this.state
55+
);
56+
57+
/**
58+
* Determine if this update will cause an actual state change.
59+
* We only want to mark as "interacted" if the state is changing.
60+
*/
61+
const accordionGroup = this.accordionGroupEl;
62+
if (accordionGroup) {
63+
const value = accordionGroup.value;
64+
const accordionValue = this.value;
65+
const shouldExpand = Array.isArray(value) ? value.includes(accordionValue) : value === accordionValue;
66+
const isExpanded = this.state === AccordionState.Expanded || this.state === AccordionState.Expanding;
67+
const stateWillChange = shouldExpand !== isExpanded;
68+
69+
console.log(
70+
'[Accordion]',
71+
this.value,
72+
'shouldExpand:',
73+
shouldExpand,
74+
'isExpanded:',
75+
isExpanded,
76+
'stateWillChange:',
77+
stateWillChange
78+
);
79+
80+
/**
81+
* If this is not an initial update AND the state is actually changing,
82+
* mark that we've had an interaction. This prevents redundant updates
83+
* (like React StrictMode re-renders) from incorrectly enabling animations.
84+
*/
85+
if (!initialUpdate && stateWillChange) {
86+
console.log('[Accordion]', this.value, 'Setting hasInteracted to true');
87+
this.hasInteracted = true;
88+
}
89+
}
90+
4491
this.updateState(initialUpdate);
4592
};
4693
private contentEl: HTMLDivElement | undefined;
@@ -55,12 +102,18 @@ export class Accordion implements ComponentInterface {
55102
@State() isNext = false;
56103
@State() isPrevious = false;
57104
/**
58-
* Tracks whether the component has completed its initial render.
59-
* Animations are disabled until after the first render completes.
60-
* This prevents the accordion from animating when it starts
61-
* expanded or collapsed on initial load.
105+
* Tracks whether a user-initiated interaction has occurred.
106+
* Animations are disabled until the first interaction happens.
107+
* This prevents the accordion from animating when it's programmatically
108+
* set to an expanded or collapsed state on initial load.
109+
*/
110+
@State() hasInteracted = false;
111+
112+
/**
113+
* Tracks if this accordion has ever been expanded.
114+
* Used to prevent the first expansion from animating.
62115
*/
63-
@State() hasRendered = false;
116+
private hasEverBeenExpanded = false;
64117

65118
/**
66119
* The value of the accordion. Defaults to an autogenerated
@@ -130,18 +183,6 @@ export class Accordion implements ComponentInterface {
130183
});
131184
}
132185

133-
componentDidRender() {
134-
/**
135-
* After the first render completes, mark that we've rendered.
136-
* Setting this state property triggers a re-render, at which point
137-
* animations will be enabled. This ensures animations are disabled
138-
* only for the initial render, avoiding unwanted animations on load.
139-
*/
140-
if (!this.hasRendered) {
141-
this.hasRendered = true;
142-
}
143-
}
144-
145186
private setItemDefaults = () => {
146187
const ionItem = this.getSlottedHeaderIonItem();
147188
if (!ionItem) {
@@ -236,21 +277,37 @@ export class Accordion implements ComponentInterface {
236277
};
237278

238279
private expandAccordion = (initialUpdate = false) => {
280+
console.log('[Accordion]', this.value, 'expandAccordion - initialUpdate:', initialUpdate, 'state:', this.state);
239281
const { contentEl, contentElWrapper } = this;
240282
if (initialUpdate || contentEl === undefined || contentElWrapper === undefined) {
241283
this.state = AccordionState.Expanded;
284+
/**
285+
* Mark that this accordion has been expanded at least once.
286+
* Do this even on initial expansion so future interactions animate.
287+
*/
288+
this.hasEverBeenExpanded = true;
289+
console.log('[Accordion]', this.value, 'expandAccordion early return - hasEverBeenExpanded set to true');
242290
return;
243291
}
244292

245293
if (this.state === AccordionState.Expanded) {
294+
console.log('[Accordion]', this.value, 'expandAccordion - already expanded, returning');
246295
return;
247296
}
248297

249298
if (this.currentRaf !== undefined) {
250299
cancelAnimationFrame(this.currentRaf);
251300
}
252301

302+
/**
303+
* Mark that this accordion has been expanded at least once.
304+
* This allows subsequent expansions to animate.
305+
*/
306+
this.hasEverBeenExpanded = true;
307+
console.log('[Accordion]', this.value, 'expandAccordion - hasEverBeenExpanded set to true');
308+
253309
if (this.shouldAnimate()) {
310+
console.log('[Accordion]', this.value, 'expandAccordion - will animate');
254311
raf(() => {
255312
this.state = AccordionState.Expanding;
256313

@@ -315,11 +372,23 @@ export class Accordion implements ComponentInterface {
315372
*/
316373
private shouldAnimate = () => {
317374
/**
318-
* Don't animate until after the first render cycle completes.
375+
* Don't animate until after the first user interaction.
319376
* This prevents animations on initial load when accordions
320-
* start in an expanded or collapsed state.
377+
* start in an expanded or collapsed state programmatically.
378+
*
379+
* Additionally, don't animate the very first expansion even if
380+
* hasInteracted is true. This handles edge cases like React StrictMode
381+
* where effects run twice and might incorrectly mark as interacted.
321382
*/
322-
if (!this.hasRendered) {
383+
if (!this.hasInteracted || !this.hasEverBeenExpanded) {
384+
console.log(
385+
'[Accordion]',
386+
this.value,
387+
'shouldAnimate: false - hasInteracted:',
388+
this.hasInteracted,
389+
'hasEverBeenExpanded:',
390+
this.hasEverBeenExpanded
391+
);
323392
return false;
324393
}
325394

@@ -418,6 +487,12 @@ export class Accordion implements ComponentInterface {
418487

419488
if (disabled || readonly) return;
420489

490+
/**
491+
* Mark that the user has interacted with the accordion.
492+
* This enables animations for all future state changes.
493+
*/
494+
this.hasInteracted = true;
495+
421496
if (accordionGroupEl) {
422497
/**
423498
* Because the accordion group may or may
@@ -438,6 +513,20 @@ export class Accordion implements ComponentInterface {
438513
const expanded = this.state === AccordionState.Expanded || this.state === AccordionState.Expanding;
439514
const headerPart = expanded ? 'header expanded' : 'header';
440515
const contentPart = expanded ? 'content expanded' : 'content';
516+
const shouldAnimate = this.shouldAnimate();
517+
518+
console.log(
519+
'[Accordion]',
520+
this.value,
521+
'render - state:',
522+
this.state,
523+
'shouldAnimate:',
524+
shouldAnimate,
525+
'hasInteracted:',
526+
this.hasInteracted,
527+
'hasEverBeenExpanded:',
528+
this.hasEverBeenExpanded
529+
);
441530

442531
this.setAria(expanded);
443532

@@ -456,7 +545,7 @@ export class Accordion implements ComponentInterface {
456545
'accordion-disabled': disabled,
457546
'accordion-readonly': readonly,
458547

459-
'accordion-animated': this.shouldAnimate(),
548+
'accordion-animated': shouldAnimate,
460549
}}
461550
>
462551
<div

0 commit comments

Comments
 (0)