smastrom / vue-collapsed

🏋️‍♂️ CSS height transition from any to auto and vice versa for Vue and Nuxt. Accordion ready.
https://vue-collapsed.pages.dev
MIT License
123 stars 8 forks source link

Collapse transition never finishing for custom zoom of the page #26

Closed krzysztof-krzeszewski closed 2 months ago

krzysztof-krzeszewski commented 2 months ago

In the code the onCollapse function is called only when the following condition is satisfied:

    if (baseHeight.value === getComputedHeight(collapseRef)) {
        onCollapsed()
    }

However the computed height of the element is not always exact.

For instance I have an element that is 42px in height. When running in chrome, when user zooms the page out to 90% the computed height value is no longer 42px but 41.9965px. So this condition fails, and the state of the collapsible is never updated.

This is a subpixel rendering issue that caused it, so I think a proper solution would be to trim/round the computed value. I don't think a sub-pixel precision is required and it would fix this issue.

On an unrelated note: floating point numbers shouldn't be compared using equality operator.

smastrom commented 2 months ago

Hi @krzysztof-krzeszewski, thanks for raising this issue. Although I couldn't reproduce this on lastest Chrome version on macOS (the scaling is at 75% in the video below but I tried with any other percentage):

https://github.com/smastrom/vue-collapsed/assets/60471784/ab8b711d-b873-46f9-b971-6bfbc0719f0a

You're correct and this should definitely be made bullet-proof. Already working on it.

krzysztof-krzeszewski commented 2 months ago

You are correct, sorry I misreported the issue. I meant the onExpanded function not the onCollapsed actually. Cause onCollapsed is meant to trigger when it reaches 0.

I think your demo is working fine, because height is not explicitly set, but I may be mistaken. Either way I used your demo project, explicitly set the height of the content and on newest version of chrome I have an expand event that never finishes when zoomed out to 90%. explicit-height

smastrom commented 2 months ago

Ah ok I see, please let me know if it works as expected here: https://1-3-2.vue-collapsed.pages.dev/

krzysztof-krzeszewski commented 2 months ago

It's working fine for me.