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

fix(accordion-group): skip initial animation #30729

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ShaneK wants to merge 8 commits into main
base: main
Choose a base branch
Loading
from FW-6730
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
View file Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
export interface AccordionGroupChangeEventDetail<T = any> {
value: T;
initial?: boolean;
}

export interface AccordionGroupCustomEvent<T = any> extends CustomEvent {
Expand Down
74 changes: 45 additions & 29 deletions core/src/components/accordion-group/accordion-group.tsx
View file Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,33 +73,13 @@ export class AccordionGroup implements ComponentInterface {
*/
@Event() ionValueChange!: EventEmitter<AccordionGroupChangeEventDetail>;

private hasEmittedInitialValue = false;
private isUserInitiatedChange = false;

@Watch('value')
valueChanged() {
const { value, multiple } = this;

if (!multiple && Array.isArray(value)) {
/**
* We do some processing on the `value` array so
* that it looks more like an array when logged to
* the console.
* Example given ['a', 'b']
* Default toString() behavior: a,b
* Custom behavior: ['a', 'b']
*/
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 });
this.emitValueChange(false, this.isUserInitiatedChange);
this.isUserInitiatedChange = false;
}

@Watch('disabled')
Expand Down Expand Up @@ -184,11 +164,12 @@ export class AccordionGroup implements ComponentInterface {
* it is possible for the value to be set after the Web Component
* initializes but before the value watcher is set up in Stencil.
* As a result, the watcher callback may not be fired.
* We work around this by manually calling the watcher
* callback when the component has loaded and the watcher
* is configured.
* We work around this by manually emitting a value change when the component
* has loaded and the watcher is configured.
*/
this.valueChanged();
if (!this.hasEmittedInitialValue) {
this.emitValueChange(true);
}
}

/**
Expand All @@ -200,6 +181,7 @@ export class AccordionGroup implements ComponentInterface {
* accordion group to an array.
*/
private setValue(accordionValue: string | string[] | null | undefined) {
this.isUserInitiatedChange = true;
const value = (this.value = accordionValue);
this.ionChange.emit({ value });
}
Expand Down Expand Up @@ -276,6 +258,40 @@ export class AccordionGroup implements ComponentInterface {
return Array.from(this.el.querySelectorAll(':scope > ion-accordion')) as HTMLIonAccordionElement[];
}

private emitValueChange(initial: boolean, isUserInitiated = false) {
const { value, multiple } = this;

if (!multiple && Array.isArray(value)) {
/**
* We do some processing on the `value` array so
* that it looks more like an array when logged to
* the console.
* Example given ['a', 'b']
* Default toString() behavior: a,b
* Custom behavior: ['a', 'b']
*/
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
);
}

/**
* Track if this is the initial value update so accordions
* can skip transition animations when they first render.
*/
const shouldMarkInitial = initial || (!this.hasEmittedInitialValue && value !== undefined && !isUserInitiated);

this.ionValueChange.emit({ value, initial: shouldMarkInitial });

if (value !== undefined) {
this.hasEmittedInitialValue = true;
}
}

render() {
const { disabled, readonly, expand } = this;
const mode = getIonMode(this);
Expand Down
37 changes: 35 additions & 2 deletions core/src/components/accordion/accordion.tsx
View file Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import type { ComponentInterface } from '@stencil/core';
import { Component, Element, Host, Prop, State, Watch, h } from '@stencil/core';
import { Component, Element, Host, Prop, State, Watch, forceUpdate, h } from '@stencil/core';
import { addEventListener, getElementRoot, raf, removeEventListener, transitionEndAsync } from '@utils/helpers';
import { chevronDown } from 'ionicons/icons';

import { config } from '../../global/config';
import { getIonMode } from '../../global/ionic-global';
import type { AccordionGroupChangeEventDetail } from '../accordion-group/accordion-group-interface';

const enum AccordionState {
Collapsed = 1 << 0,
Expand Down Expand Up @@ -38,12 +39,23 @@ const enum AccordionState {
})
export class Accordion implements ComponentInterface {
private accordionGroupEl?: HTMLIonAccordionGroupElement | null;
private updateListener = () => this.updateState(false);
private updateListener = (ev: CustomEvent<AccordionGroupChangeEventDetail>) => {
const initialUpdate = ev.detail?.initial ?? false;
this.updateState(initialUpdate);
};
private contentEl: HTMLDivElement | undefined;
private contentElWrapper: HTMLDivElement | undefined;
private headerEl: HTMLDivElement | undefined;

private currentRaf: number | undefined;
/**
* If true, the next animation will be skipped.
* We want to skip the animation when the accordion
* is expanded or collapsed on the initial load.
* This prevents the accordion from animating when
* it starts expanded or collapsed.
*/
private skipNextAnimation = true;

@Element() el?: HTMLElement;

Expand Down Expand Up @@ -119,6 +131,18 @@ export class Accordion implements ComponentInterface {
});
}

componentDidRender() {
if (this.skipNextAnimation) {
this.skipNextAnimation = false;
/**
* The initial render disables animations so framework-provided
* values do not cause the accordion to animate. Force a repaint
* so subsequent user interactions animate as expected.
*/
forceUpdate(this);
}
}

private setItemDefaults = () => {
const ionItem = this.getSlottedHeaderIonItem();
if (!ionItem) {
Expand Down Expand Up @@ -291,6 +315,10 @@ export class Accordion implements ComponentInterface {
* of what is set in the config.
*/
private shouldAnimate = () => {
if (this.skipNextAnimation) {
return false;
}

if (typeof (window as any) === 'undefined') {
return false;
}
Expand Down Expand Up @@ -323,6 +351,11 @@ export class Accordion implements ComponentInterface {
const value = accordionGroup.value;

const shouldExpand = Array.isArray(value) ? value.includes(accordionValue) : value === accordionValue;
const shouldDisableAnimation = initialUpdate && shouldExpand;

if (shouldDisableAnimation) {
this.skipNextAnimation = true;
}

if (shouldExpand) {
this.expandAccordion(initialUpdate);
Expand Down
103 changes: 103 additions & 0 deletions core/src/components/accordion/test/accordion.spec.ts
View file Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { newSpecPage } from '@stencil/core/testing';

import type { AccordionGroupChangeEventDetail } from '../../accordion-group/accordion-group-interface';
import { AccordionGroup } from '../../accordion-group/accordion-group';
import { Item } from '../../item/item';
import { Accordion } from '../accordion';
Expand Down Expand Up @@ -200,6 +201,108 @@ 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>
`;

const details: AccordionGroupChangeEventDetail[] = [];
accordionGroup.addEventListener('ionValueChange', (event: CustomEvent<AccordionGroupChangeEventDetail>) => {
details.push(event.detail);
});

accordionGroup.value = 'first';
page.body.appendChild(accordionGroup);

await page.waitForChanges();

expect(details[0]?.initial).toBe(true);

const firstAccordion = accordionGroup.querySelector('ion-accordion[value="first"]')!;

expect(firstAccordion.classList.contains('accordion-expanded')).toEqual(true);
expect(firstAccordion.classList.contains('accordion-expanding')).toEqual(false);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't realize that spec tests capture these kinds of classes. I would have expected it to be false all the time since expect would run after the animation ended. I'll have to keep this in mind for future tests. Thanks!

ShaneK reacted with rocket emoji
});

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>
`;

const details: AccordionGroupChangeEventDetail[] = [];
accordionGroup.addEventListener('ionValueChange', (event: CustomEvent<AccordionGroupChangeEventDetail>) => {
details.push(event.detail);
});

page.body.appendChild(accordionGroup);
await page.waitForChanges();

accordionGroup.value = 'first';
await page.waitForChanges();

const firstDetail = details.find((detail) => detail.value === 'first');
expect(firstDetail?.initial).toBe(true);

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 animate when accordion is first opened by user', 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 details: AccordionGroupChangeEventDetail[] = [];
accordionGroup.addEventListener('ionValueChange', (event: CustomEvent<AccordionGroupChangeEventDetail>) => {
details.push(event.detail);
});

await accordionGroup.requestAccordionToggle('first', true);
await page.waitForChanges();

const lastDetail = details[details.length - 1];
expect(lastDetail?.initial).toBe(false);

const firstAccordion = accordionGroup.querySelector('ion-accordion[value="first"]')!;
expect(firstAccordion.classList.contains('accordion-animated')).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({
Expand Down
2 changes: 2 additions & 0 deletions packages/react/test/base/src/App.tsx
View file Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down Expand Up @@ -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>
Expand Down
54 changes: 54 additions & 0 deletions packages/react/test/base/src/pages/AccordionGroup.tsx
View file Open in desktop
Original file line number Diff line number Diff line change
@@ -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;
3 changes: 3 additions & 0 deletions packages/react/test/base/src/pages/Main.tsx
View file Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
Loading

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