Skip to content
This repository has been archived by the owner on Aug 19, 2022. It is now read-only.

Bug: Render array #979

Merged
merged 9 commits into from
Mar 29, 2018
Merged

Bug: Render array #979

merged 9 commits into from
Mar 29, 2018

Conversation

chrisbolin
Copy link
Contributor

@chrisbolin chrisbolin commented Mar 28, 2018

React 16 introduced rendering arrays of elements.

render() {
  return [
    <li key="A">First item</li>,
    <li key="B">Second item</li>,
    <li key="C">Third item</li>,
  ];
}

This usage threw an error in Radium: #950

@chrisbolin
Copy link
Contributor Author

Note that Radium requires the use of keys. From the Guide...

Radium allows you to style multiple elements in the same component. You just have to give each element that has browser state modifiers like :hover or media queries a unique key or ref attribute.

That requirement still holds true with rendered arrays. (In fact, React requires keys for rendered arrays)

@coveralls
Copy link

coveralls commented Mar 28, 2018

Coverage Status

Coverage increased (+0.06%) to 94.616% when pulling fdd037b on bug/render-array into 26034a3 on master.

@chrisbolin
Copy link
Contributor Author

The next step (can be in a different PR) will be testing <Fragment>, which was introduced in React 16.2. We'll need to update yarn.lock to 16.2.0

Copy link
Member

@ryan-roemer ryan-roemer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One minor nit, rest LGTM.

@@ -385,6 +385,34 @@ resolveStyles = function(
);
}

// TODO HERE
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's clean up all TODO notes 😉

Copy link
Contributor

@alexlande alexlande left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

excellent 🤘

@chrisbolin
Copy link
Contributor Author

Merging. Will tackle testing <Fragment> rendering next (and fix things if necessary)

@chrisbolin chrisbolin closed this Mar 29, 2018
@chrisbolin chrisbolin reopened this Mar 29, 2018
@chrisbolin chrisbolin merged commit 7ee95cb into master Mar 29, 2018
@chrisbolin chrisbolin deleted the bug/render-array branch March 29, 2018 01:36
jaredbrookswhite pushed a commit to jaredbrookswhite/radium that referenced this pull request Aug 22, 2018
Bug fix: render an array of elements

* handle array child elements; ensure that extraStateKeyMap is cleaned up

* Add regression test for render children.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants