Skip to content
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

bug: accordion doesn't take into account content margin during expand animation #26381

Closed
4 of 7 tasks
kemp opened this issue Nov 30, 2022 · 2 comments · Fixed by #26390
Closed
4 of 7 tasks

bug: accordion doesn't take into account content margin during expand animation #26381

kemp opened this issue Nov 30, 2022 · 2 comments · Fixed by #26390
Labels
package: core @ionic/core package type: bug a confirmed bug report

Comments

@kemp
Copy link
Contributor

kemp commented Nov 30, 2022

Prerequisites

Ionic Framework Version

  • v4.x
  • v5.x
  • v6.x
  • Nightly

Current Behavior

When expanding an accordion that contains an item with a margin (such as ion-card) the animation is jerky because the max height does not include the margin. The issue is exacerbated when there are multiple cards within the accordion.

Expected Behavior

The animation should smoothly complete.

Steps to Reproduce

Create an ion-accordion-group and ion-accordion with an ion-card inside. Open the accordion and notice that the items below start to smoothly animate but jerk into place in the last instant.

Code Reproduction URL

https://stackblitz.com/edit/ionic6-angular13-ttmnlj?file=src/app/app.component.html

Ionic Info

Ionic:

Ionic CLI : 6.20.1 (/usr/local/lib/node_modules/@ionic/cli)
Ionic Framework : @ionic/angular 6.3.8
@angular-devkit/build-angular : 14.2.10
@angular-devkit/schematics : 14.2.10
@angular/cli : 14.2.10
@ionic/angular-toolkit : 6.1.0

Capacitor:

Capacitor CLI : 4.0.1
@capacitor/android : 4.5.0
@capacitor/core : 4.5.0
@capacitor/ios : 4.5.0

Utility:

cordova-res : 0.15.4
native-run : 1.7.1

System:

NodeJS : v16.18.1 (/home/work/.nvm/versions/node/v16.18.1/bin/node)
npm : 8.19.2
OS : Linux 6.0

Additional Information

The suspect source code line:

const contentHeight = contentElWrapper.offsetHeight;

An answer on StackOverflow to take into account margins of an element: https://stackoverflow.com/a/23749355/2544756

When searching for issues, I found this, but it's not related: #25879

I can work on a PR for this, I just want to confirm that it is something we want fixed.

@ionitron-bot ionitron-bot bot added the triage label Nov 30, 2022
@sean-perkins sean-perkins self-assigned this Nov 30, 2022
@sean-perkins
Copy link
Contributor

Hello @kemp thank you for reporting this issue!

We would happily welcome a PR contribution to resolve this issue. The animation would preferably be smooth when animating with content inside of it that includes a margin. We would just want to validate that taking additional measurements into the calculation of the max height (contentHeight) does not regress the animation in other scenarios.

@sean-perkins sean-perkins added package: core @ionic/core package type: bug a confirmed bug report labels Nov 30, 2022
@sean-perkins sean-perkins removed their assignment Nov 30, 2022
@ionitron-bot ionitron-bot bot removed the triage label Nov 30, 2022
kemp pushed a commit to kemp/ionic-framework that referenced this issue Nov 30, 2022
@ionitron-bot
Copy link

ionitron-bot bot commented Apr 12, 2023

Thanks for the issue! This issue is being locked to prevent comments that are not relevant to the original issue. If this is still an issue with the latest version of Ionic, please create a new issue and ensure the template is fully filled out.

@ionitron-bot ionitron-bot bot locked and limited conversation to collaborators Apr 12, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
package: core @ionic/core package type: bug a confirmed bug report
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants