Add Collapse

[Chalarangelo]

A simple collapse component:

• Uses state.
• Binds an event listener callback to its own context.
• Uses props.children to render content.
• Is accessible via aria-expanded.
• Has some CSS-in-JS (as unopinionated and minimal as possible).

[Chalarangelo]

Linecount aside, I think this turned out pretty good. Opinions @skatcat31 @fejes713?

[skatcat31]

All good except for deriving state at PIT on an async action

[Chalarangelo]

@skatcat31 Already fixed - I used GitHub's suggestion feature to use your code, so one of the commits is on you now! 😉 - Please use that feature whenever possible, it helps us improve PRs faster and improves git history/blame.

[skatcat31]

@skatcat31 Already fixed - I used GitHub's suggestion feature to use your code, so one of the commits is on you now! 😉 - Please use that feature whenever possible, it helps us improve PRs faster and improves git history/blame.

I'll have to check into that feature, haven't noticed it quite yet

[skatcat31]

Original comment by @skatcat31

Shouldn't this use a div wrapping the button and context that is collapsible, not a fragment?

While a fragment should work, it'll mean that where Collapse is called will nest it's children on the parent directly, not under a div(IIRC)

reply by @Chalarangelo

Yeah, I usually opt for not adding extra elements for no reason in components, but this might be a good case against fragments actually.

reply by @Chalarangelo

Removing the Fragment will also probably drop expertise to 1 instead of 2 and make reading this a bit easier.

I wanna test out that suggestions thing

edit: umm... I can't seem to find it. I wonder if it only shows up if you're the branch author??

edit2: darn seems to be to short for it to detect easily...

edit3: OHHH

[Chalarangelo]

@skatcat31 I think this is alright now.

[skatcat31]

Agreed however I think it should be left as a 2 since it's nesting context and using children

[Chalarangelo]

Alright, I think there is no controversy surrounding this, so I'm gonna merge the first ever snippet into the master branch! 🚀

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for any follow-up tasks.