fix: Improve ModalActions behaviour with null child items

[scottlovegrove]

Closes #613

Short description

This change ensures that if child items passed into the ModalActions component are null, then they do not have an empty div, with spacing, created. It also ensures that if all child items passed into the component are null, then just don't create the footer in the first place.

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

This is not a breaking change, but I haven't updated the version.

[henningmu]

I don't think we need to check here for falsy values as we filter them out above already. All child should be truthy 👍

---

A question for @gnapse: why do we need to wrap the children in divs in the first place? Shouldn't Inline work with anything we throw at it? And if not should we pass this burden to the user?

[scottlovegrove]

I don't think we need to check here for falsy values as we filter them out above already. All child should be truthy 👍

Say hello to the original fix 😄 I'll remove that check now.

[gnapse]

why do we need to wrap the children in divs in the first place? Shouldn't Inline work with anything we throw at it? And if not should we pass this burden to the user?

Nope. This was a compromise to avoid having Inline and Stack generate excessive markup all the time. See https://github.com/Doist/reactist/wiki/Simplified-markup-of-the-Stack-and-Inline-components

[henningmu]

Makes sense and awesome that we have documentation on this ❤️

[gnapse]

Thanks for this, Scott. It certainly is an oversight.

However, this has made me realize that there's another related oversight that we might as well fix alongside the original issue. Imagine something like this:

<ModalActions>
  {canSubmit ? (
    <>
      <Button type="submit">Delete one</Button>
      <Button type="submit">Delete all</Button>
    </>
  ) : null}
  <Button>Cancel</Button>
</ModalActions>

The first two buttons, being wrapped by a fragment, will be part of a single child of the Inline wrapper, thus lacking a spacing between them. What a consume of this component would expect is that the above ModalAction is treated as a Inline with 3 spaced-out children when the canSubmit condition is true.

A better fix for this issue goes along the lines of using react-keyed-flatten-children. An example of this is present in the Stack component implementation.

Since Scott is on time off for a few weeks, I may take this over and do the change myself.

[gnapse]

Closed without merging, superseded by #615.