fix: Various fixes to the ModalActions component

[gnapse]

Closes #613
Supersedes PR #614

Short description

This PR performs the following changes on the ModalActions component:

• ignore null children
• flatten children wrapped in React fragments
• removes the unneeded extra div wrapping each child element (following the same logic described here).

Additionally, this removes an extra Box wrapping the main element rendered by Inline. I was surprised to see it, and did several manual testing (both in the storybook, and running twist-web with my local version of Reactist with these changes in place) and, as I suspected, this outer div is not needed.

PR Checklist

• Added tests for bugs / new features
• Updated docs (storybooks, readme)
• Executed npm run validate and made sure no errors / warnings were shown
• Described changes in CHANGELOG.md
• Bumped version in package.json and package-lock.json (npm --no-git-tag-version version <major|minor|patch>) ref
• Updated all static build artifacts (npm run build-all)

Versioning

Queued, to be included in an upcoming release.

[nats12]

Wouldn't this make it harder to debug null values as a consumer? Say we are passing down a div containing a null value. We would at least be able to see that the div is correctly being rendered, it just doesn't have a value. Not against it though, just wanted to learn more regarding the decision to ignore them

[gnapse]

Say we are passing down a div containing a null value.

What do you mean with "a div containing a null value"? And what do you mean with "debug null values as a consumer"?

[nats12]

What I understand from the above is that if we have:

<ModalActions data-testid="modal-actions">
                <button>{some null value}</button>
</ModalActions>

We would see:

<ModalActions data-testid="modal-actions">
                <button>OK</button>
                <button>Cancel</button>
 </ModalActions>

Is that right? Or would we render a third empty button tag?

And what do you mean with "debug null values as a consumer"?

If I've understood it right and the above example is how it would work, wouldn't it be confusing to eliminate the empty button tag with the null value? Especially as we inspect the DOM to see why it hasn't rendered?

[gnapse]

What the above example shows is that if one of the direct children of ModalActions is null, it will not generate a "slot" in the sequence of inlined elements that are spaced out. It has nothing to do with having non-null children that have their children null.

If this component that we're testing receives a <button>{some null value}</button>, it has no way of knowing that the component has null children. It only knows if its own children are null.

[nats12]

Looks good! Just left a question around the ignoring of null children, but nothing that's blocking

[henningmu]

@gnapse let's merge this?

[gnapse]

@gnapse let's merge this?

Thanks for the reminder, I had forgotten about this.

I'll do a smoke test running Twist locally against this branch, to make sure that it does not break anything. I think I did this at some point, but I'm not 100% sure.

[gnapse]

I found no major consequences. The only noticeable thing is that under certain situations (e.g. a Inline directly inside a Stack) we may need to manually wrap the Inline inside a div or Box. I only found one instance of this happening in Twist, and in any case the actual visual consequence is almost not noticeable.

This is an expected consequence of the removal of this extra wrapper, as documented here.

I'll fix it in Twist in the PR that will upgrade to the new Reactist version, which I'll create this week.