fix(accordion): include margins during expand animation

[kemp]

Closes #26381

It appears that the issue had nothing to do with the calculation of the height in the script, but instead was due to collapsing margins. Adding overflow: auto prevents the margins from collapsing.

Pull request checklist

Please check if your PR fulfills the following requirements:

• Tests for the changes have been added (for bug fixes / features)
• Docs have been reviewed and added / updated if needed (for bug fixes / features)
  • Some docs updates need to be made in the ionic-docs repo, in a separate PR. See the contributing guide for details.
• Build (npm run build) was run locally and any changes were pushed
• Lint (npm run lint) has passed locally and any fixes were made for failures

Pull request type

Please check the type of change your PR introduces:

• Bugfix
• Feature
• Code style update (formatting, renaming)
• Refactoring (no functional changes, no api changes)
• Build related changes
• Documentation content changes
• Other (please describe):

What is the current behavior?

The animation for accordions is jerky when the root content element has a margin.

Issue URL: #26381

What is the new behavior?

• The animation completes smoothly.

Does this introduce a breaking change?

• Yes
• No

Other information

N/A

[liamdebeasi]

Thanks for the PR! It looks like this breaks some tests. Can you look into fixing the code such that the tests pass? Let me know if you have questions about any of the tests.

[kemp]

It looks like I just had a duplicate id in the test. Should be fixed now

[kemp]

It seems like the failures are not related to this change?

[liamdebeasi]

Hmm yeah looks like the test runner is having issues:

browserContext.close: Protocol error (Browser.removeBrowserContext): Browser closed.
    ==================== Browser output: ====================
    <launching> /home/runner/.cache/ms-playwright/firefox-1327/firefox/firefox -no-remote -headless -profile /tmp/playwright_firefoxdev_profile-hMNXk4 -juggler-pipe -silent
    <launched> pid=10760
    [pid=10760][err] *** You are running in headless mode.
    [pid=10760][out] Crash Annotation GraphicsCriticalError: |[0][GFX1-]: glxtest: Unable to open a connection to the X server (t=0.277242) [GFX1-]: glxtest: Unable to open a connection to the X server
    [pid=10760][out] Crash Annotation GraphicsCriticalError: |[0][GFX1-]: glxtest: Unable to open a connection to the X server (t=0.277242) |[1][GFX1-]: glxtest: libEGL initialize failed (t=0.277295) [GFX1-]: glxtest: libEGL initialize failed
    [pid=10760][out] 
    [pid=10760][out] Juggler listening to the pipe
    [pid=10760][out] console.warn: SearchSettings: "get: No settings file exists, new profile?" (new NotFoundError("Could not open the file at /tmp/playwright_firefoxdev_profile-hMNXk4/search.json.mozlz4", (void 0)))
    [pid=10760][out] Crash Annotation GraphicsCriticalError: |[0][GFX1-]: glxtest: Unable to open a connection to the X server (t=0.277242) |[1][GFX1-]: glxtest: libEGL initialize failed (t=0.277295) |[2][GFX1-]: RenderCompositorSWGL failed mapping default framebuffer, no dt (t=1.54857) [GFX1-]: RenderCompositorSWGL failed mapping default framebuffer, no dt
    [pid=10760][err] JavaScript error: resource://gre/modules/XULStore.jsm, line 66: Error: Can't find profile directory.
    [pid=10760][err] JavaScript error: resource://gre/modules/XULStore.jsm, line 66: Error: Can't find profile directory.
    [pid=10760][out] console.warn: LoginRecipes: "getRecipes: falling back to a synchronous message for:" "http://localhost:3333"
    [pid=10760][out] console.warn: LoginRecipes: "getRecipes: falling back to a synchronous message for:" "http://localhost:3333"
    [pid=10760][out] console.warn: LoginRecipes: "getRecipes: falling back to a synchronous message for:" "http://localhost:3333"
    [pid=10760][err] Exiting due to channel error.
    [pid=10760][err] Exiting due to channel error.

I'll rerun the failing tests and see if that fixes it.

[liamdebeasi]

I am still able to reproduce the issue when checking out your code:

Screen.Recording.2022-12-09.at.2.55.44.PM.mov

tested on Chrome 107 on macOS 12.6

[kemp]

Ah, good ol' browser discrepancies. I'm running linux and unfortunately do not have access to a macOS device.

I tested Chrome Version 107.0.5304.110 (Official Build) (64-bit) as well as Firefox 107.0 (64-bit) and the animation appears smooth.

Are you perhaps able to confirm that the issue is fixed on a non-macOS device to determine if the issue is only present there?

Edit: here's a video capture from my machine:

2022-12-09.15-38-04.mp4

[liamdebeasi]

I can confirm that the changes work well in a Chromium browser on Windows 10:

accordion-windows.mp4

However, the issue also persists on Chrome for Android:

accordion-android.mp4

[liamdebeasi]

Hey @kemp,

Are you still interested in working on a fix for this? In terms of remaining work, we'll need to find a solution that works cross platform.

[kemp]

@liamdebeasi yes I'm still looking into a solution when I have time. Apologies for the delay

[liamdebeasi]

No problem at all. Let me know if I can help!

[kemp]

@liamdebeasi your comment did remind me though to look at it again, lol

Could you actually try it again on your devices? I was able to test on more devices, including the ones you commented previously, and it all appears to be working for me. Perhaps there was a stale cache?

[liamdebeasi]

The fix seems to work on iOS 16. @amandaejohnston @sean-perkins Could you test on your devices too? (desktop and mobile)

[averyjohnston]

Looks good on my end 👍 Tested on Windows Chrome and Firefox, plus Android Chrome. Thank you for fixing this!

[liamdebeasi]

I previously tested this, and it seems to work. @sean-perkins Can you give this a test?

[sean-perkins]

Tested on:

• Chrome: Version 110.0.5481.177 (Official Build) (arm64)
• Mobile Safari (iOS 16.0.1)

Animation/margins looks good 👍

[kemp]

Thanks everyone. 💯