fix: ensure height does not exceed initial height

[hieunm156]

I have made some changes to ensure that the height does not exceed the initial height in the Alpine Collapse plugin.

This adjustment is based on personal user experience and #5739.

Thank you!

[calebporzio]

PR Review: #4547 — fix: ensure height does not exceed initial height

Type: Bug fix (debatable — this is arguably a behavior change)
Verdict: Close

What's happening (plain English)

The collapse plugin has a .min modifier that lets you set a minimum visible height when collapsed. For example, x-collapse.min.100px means "when collapsed, show 100px of the element as a preview."

This PR addresses a scenario where the element's natural content height is shorter than the configured min-height. For example: content is 50px tall, but you set x-collapse.min.100px. Currently, the collapsed state shows 100px — meaning 50px of content plus 50px of empty space. The PR tries to cap the collapsed height at the content's natural height so you never see empty space below the content.

The actual functional change is a single line:

Before:

if (! el._x_isShown) el.style.height = `${floor}px`

After:

let initialHeight = el.getBoundingClientRect().height
if (!el._x_isShown) el.style.height = initialHeight < floor ? `${initialHeight}px` : `${floor}px`

The rest of the diff (~90% of it) is cosmetic formatting changes: removing spaces after !, adding spaces inside () => { }, adding spaces around +.

Other approaches considered

1. Don't change anything — The current behavior is arguably correct. You asked for min.100px, you get 100px. If your content is shorter, pick a smaller min value. This is the simplest approach and keeps the API semantics clear.
2. Cap floor globally with Math.min(floor, naturalHeight) — Would fix both initial render AND subsequent collapses. But makes min a misleading name since it would no longer always be the minimum.
3. Use a max modifier instead — If users need this, a separate x-collapse.max.100px modifier or a x-collapse.min.50px.max.100px pattern would be clearer than silently capping the min. But this adds API surface.

Changes Made

No changes made.

Test Results

• Existing collapse tests: 5/5 passing
• CI status: passing
• PR adds no new tests

Code Review

Issues found:

1. Incomplete fix (packages/collapse/src/index.js:19): The height capping only applies on initial render. When the element expands and then collapses again via the out() transition (line 72), it still transitions to floor height: end: { height: floor+'px' }. So the "fix" only works on first page load, not on subsequent toggles.

2. No tests — A bug fix PR with no tests and no way to verify the claimed fix works (or continues to work).

3. ~90% noise in the diff — The vast majority of changes are cosmetic formatting: removing spaces after ! operators, adding spaces inside empty arrow functions, adding spaces around +. These reformatting changes actually violate Alpine's existing code style — Alpine uses ! with a space after the negation operator, and () => {} without spaces inside empty function bodies. The PR reformats these to ! and () => { }.

4. initialHeight captured once at init — The height measurement is taken once when the directive initializes. If content is dynamic (loaded via AJAX, conditionally rendered, etc.), this stale value will produce incorrect behavior.

5. Debatable whether this is a bug — x-collapse.min.100px means "collapsed height is 100px." If your content is shorter than 100px, the current behavior (showing 100px) is doing exactly what was requested. The "fix" changes the semantic from "minimum height" to "minimum height, but never more than the content."

Security

No security concerns identified.

Verdict

Close. Here's why:

• This is a behavior change dressed as a bug fix. The current min behavior is working as designed — it sets a minimum collapsed height. If users pick a min that's taller than their content, that's a configuration mistake, not a bug.
• The fix is incomplete — it only applies on initial render, not on subsequent collapse toggles.
• No tests are provided to verify the claimed behavior.
• The diff is ~90% cosmetic formatting noise that violates Alpine's existing code style.
• Zero community engagement (no reactions, no comments, no issue with upvotes).
• The linked discussion (x-collapse dynamic .min modifier #2984) is about a completely different problem (dynamically adding x-collapse attributes via init(), which is not how Alpine directives are intended to work).

If this behavior is actually desired, it would need to be implemented as a proper feature with:

• Consistent behavior on initial render AND subsequent collapses
• A clear test case
• No style violations or formatting noise
• Consideration of whether min is the right modifier name for this semantic

---

Reviewed by Claude