Stop jumping when content has margins

[selbekk]

This pull request fixes a bug that's been causing a lot of issues - namely the margins of collapsed content not being included when the height is being calculated.

This is done by introducing what's called a temporary new block formatting context to the content when it's not stable and opened. If that's a new CSS concept to you, that's okay - I never heard of it either. It's basically what happens when you throw a overflow-y: auto on a DOM element The end effect is that it clears any margins, which in turn makes it possible to measure the "real" height of the content without changing a lot of margins to paddings.

This PR also changes the example CSS file so that it uses default margins instead of paddings to separate sample paragraphs.

This is a WIP, but it seems to be working. What do you think?

[nkbt]

I prefer not to deal with margins at all. Or any otger css related quirks. It was an actual decision made. So if there is a need for margin - add them to containers wrapping collaps, or to children. I have never seen a situation that cannot be solved this way so far.

[selbekk]

I understand your aversion towards margins. They're tricky, they collapse when you don't want them to, and they're a general pain to deal with.

That being said, margins are something we need to deal with at some point. A typical use case for a collapse is a bunch of paragraphs of text, and each of those have their own margin (by default). Now, I can of course change those margins to paddings, but when you're working with an existing design system that uses margins everywhere, that leads to a lot of extra modifier classes for in-collapse-content.

Another approach is to add the margin to a wrapping div - but the amount of margin I have to add is dependent on the amount and type of children, which makes it a bit hard to go down that route.

This patch lets the user choose the approach they want, without putting any constraints on how they write their CSS. I'll be more than happy to add some more tests to the examples, if there are edge cases you worry about.

[nkbt]

I believe in a lean library way: if something can be achieve inside app - then it should not be in the library. New functionality needs to be added to the lib only if it is absolutely impossible to achieve needed (and common) behaviour in the app. Which is not true in this particular case. Otherwise library will quickly become unsupportable mess, which is a common characteristic of most opensource projects, struggling to deal with many options and flags.

Possible solutions are: to wrap children with element that is then added inside Collapse. No problems with margins anymore. Or fix it with paddings. Or deal with it in many other ways that CSS allows.

[selbekk]

Thanks for the quick response :smiley:

There are undoubtedly many ways to deal with components that have margins inside collapsing component - it's definitely an issue we can handle!

However, I think including margins in the height calculation should be a core feature of the library - just like including paddings, borders and actual content height should be core features.

If there's an implementation issue, I'd be more than happy to look further into improving it. If you're sure this is something you don't want to support, feel free to close this pull request - I can just continue using my own fork in my own projects :peace_symbol: I do believe, however, that this fix or feature would be beneficial to the library users!