diff --git a/core/src/components/accordion-group/accordion-group.tsx b/core/src/components/accordion-group/accordion-group.tsx index ba5b110c8b6..a2b2f82c700 100644 --- a/core/src/components/accordion-group/accordion-group.tsx +++ b/core/src/components/accordion-group/accordion-group.tsx @@ -88,17 +88,12 @@ export class AccordionGroup implements ComponentInterface { */ printIonWarning( `[ion-accordion-group] - An array of values was passed, but multiple is "false". This is incorrect usage and may result in unexpected behaviors. To dismiss this warning, pass a string to the "value" property when multiple="false". - Value Passed: [${value.map((v) => `'${v}'`).join(', ')}] `, this.el ); } - /** - * Do not use `value` here as that will be - * not account for the adjustment we make above. - */ this.ionValueChange.emit({ value: this.value }); } diff --git a/core/src/components/accordion/accordion.tsx b/core/src/components/accordion/accordion.tsx index 92d28848d20..249f8a260cb 100644 --- a/core/src/components/accordion/accordion.tsx +++ b/core/src/components/accordion/accordion.tsx @@ -38,7 +38,40 @@ const enum AccordionState { }) export class Accordion implements ComponentInterface { private accordionGroupEl?: HTMLIonAccordionGroupElement | null; - private updateListener = () => this.updateState(false); + private updateListener = () => { + /** + * 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,7 +140,7 @@ export class Accordion implements ComponentInterface { connectedCallback() { const accordionGroupEl = (this.accordionGroupEl = this.el?.closest('ion-accordion-group')); if (accordionGroupEl) { - this.updateState(true); + this.updateState(); addEventListener(accordionGroupEl, 'ionValueChange', this.updateListener); } } @@ -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 diff --git a/core/src/components/accordion/test/accordion.spec.ts b/core/src/components/accordion/test/accordion.spec.ts index 54883002dbf..e10fdc9d279 100644 --- a/core/src/components/accordion/test/accordion.spec.ts +++ b/core/src/components/accordion/test/accordion.spec.ts @@ -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 = ` + + Label +
Content
+
+ + Label +
Content
+
+ `; + + 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 = ` + + Label +
Content
+
+ + Label +
Content
+
+ `; + + 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: ` + + + Label +
Content
+
+
+ `, + }); + + 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({ diff --git a/packages/react/test/base/src/App.tsx b/packages/react/test/base/src/App.tsx index 2f7f4a63ded..634af89f075 100644 --- a/packages/react/test/base/src/App.tsx +++ b/packages/react/test/base/src/App.tsx @@ -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 = () => ( + diff --git a/packages/react/test/base/src/pages/AccordionGroup.tsx b/packages/react/test/base/src/pages/AccordionGroup.tsx new file mode 100644 index 00000000000..ffcfaca8bd2 --- /dev/null +++ b/packages/react/test/base/src/pages/AccordionGroup.tsx @@ -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); + + useEffect(() => { + if (!accordionGroup.current) { + return; + } + + accordionGroup.current.value = ['first', 'third']; + }, []); + + return ( + + + + Accordion Group + + + + + + + First Accordion + +
+ First Content +
+
+ + + Second Accordion + +
+ Second Content +
+
+ + + Third Accordion + +
+ Third Content +
+
+
+
+
+ ); +}; + +export default AccordionGroup; diff --git a/packages/react/test/base/src/pages/Main.tsx b/packages/react/test/base/src/pages/Main.tsx index dd87350d9be..3873cd3d5b5 100644 --- a/packages/react/test/base/src/pages/Main.tsx +++ b/packages/react/test/base/src/pages/Main.tsx @@ -22,6 +22,9 @@ const Main: React.FC = () => { + + Accordion Group + Overlay Hooks