The Stack is wrapping null children with a Stack.Item, which can create layout problems

[perrupa]

Issue summary

The Stack component wraps children that evaluate to null with a Stack.Item, creating strange layout problems when some components are conditionally rendered.

Expected behavior

The Stack component wraps children that evaluate to null with a Stack.Item

Actual behavior

Components that return null are not rendered, but a Stack.Item is rendered around the null value they return.

Examples

null children are not wrapped in an Item
children that evaluate to null ARE wrapped in an Item

Specifications

• Are you using the React components? (Y/N): Y
• Polaris version number: ^3.16.0

▶ npx envinfo --system --binaries --browsers --npmPackaes react,react-dom,@shopify/polaris
npx: installed 1 in 1.864s

  System:
    OS: macOS 10.14.5
    CPU: (8) x64 Intel(R) Core(TM) i7-7700HQ CPU @ 2.80GHz
    Memory: 695.54 MB / 16.00 GB
    Shell: 5.7.1 - /usr/local/bin/zsh
  Binaries:
    Node: 8.12.0 - ~/.nvm/versions/node/v8.12.0/bin/node
    Yarn: 1.13.0 - ~/.dev/yarn/1.13.0/bin/yarn
    npm: 6.4.1 - ~/.nvm/versions/node/v8.12.0/bin/npm
    Watchman: 4.9.0 - /usr/local/bin/watchman
  Browsers:
    Chrome: 75.0.3770.142
    Chrome Canary: 77.0.3865.0
    Firefox: 66.0.3
    Safari: 12.1.1
  npmPackages:
    @shopify/polaris: ^3.16.0 => 3.16.0
    react: 16.8.6 => 16.8.6
    react-dom: ^16.8.0 => 16.8.6

Research/Best guesses?

The Item wrapping logic here uses wrapWithComponent.

I'm not sure if that's exactly where the bug lies, but my guess would be that this line here is the root of the problem. It checks that the component is null, but not whether or not it evaluates to null.

👋 Thanks for opening your first issue. A contributor should give feedback soon. If you haven’t already, please check out the contributing guidelines. You can also join #polaris on the Shopify Partners Slack.

This issue has been inactive for 180 days and labeled with Icebox. It will be closed in 7 days if there is no further activity.

This issue has been inactive for 180 days and labeled with Icebox. It will be closed in 7 days if there is no further activity.

[yishus]

👋 I'm running into a similar issue with a small example over here. Is this something that we would not be fixing?