feat: 'render' callback for shorthand props

[kuzhelov]

Implements #355.

TODO

• add/adjust unit tests
• remove render methods from component's code
• update CHANGELOG

---

Provided changes introduce additional ways to specify shorthand's value using render callback. Following examples are aimed to deliver key points.

Base case

<Button icon="book" />
<Button icon={{ name: 'book' }} />

Render primitive value

<Button icon={render => render('book')} />

Render object value

<Button icon={render => render({ name: 'book' }) } />

Render React element (no evaluated props consumed)

<Button icon={render => render(<span>▼▲</span>) } />

Render custom tree with evaluated shorthand props

Note, this is a functionality we've previously had with render methods.

<Button icon={render => render(
   'book',

  // where 'props' contains all evaluated ones from { name: 'book' }
  (Component, props) => <Component {...props} />)} 
/>

OR

<Button icon={render => render(
   { name: 'book' },

   // where 'props' contains all evaluated ones from { name: 'book' }
   (Component, props) => <Component {...props} />) } 
/>

[codecov]

Codecov Report

❗ No coverage uploaded for pull request base (master@2d1c51d). Click here to learn what that means.
The diff coverage is 100%.

@@            Coverage Diff            @@
##             master     #519   +/-   ##
=========================================
  Coverage          ?   87.96%           
=========================================
  Files             ?       42           
  Lines             ?     1404           
  Branches          ?      183           
=========================================
  Hits              ?     1235           
  Misses            ?      165           
  Partials          ?        4

Impacted Files	Coverage Δ
src/components/Menu/Menu.tsx	100% <ø> (ø)
src/components/Chat/Chat.tsx	95.65% <100%> (ø)
src/components/Form/Form.tsx	100% <100%> (ø)
src/components/List/List.tsx	61.66% <100%> (ø)
src/components/Segment/Segment.tsx	100% <100%> (ø)
src/components/Input/Input.tsx	100% <100%> (ø)
src/components/Form/FormField.tsx	100% <100%> (ø)
src/components/Header/Header.tsx	94.73% <100%> (ø)
src/components/RadioGroup/RadioGroup.tsx	95.52% <100%> (ø)
src/components/Status/Status.tsx	100% <100%> (ø)
... and 10 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2d1c51d...f20320f. Read the comment docs.

[mnajdova]

I tested it locally and it works as expected. Left two comments for consideration. I vote that we start applying the changes to all components.

[layershifter]

I want to mention that we have a case where render* is passed down to other components and this can be really useful for nested items like submenus, #512.

It's a real case:

<Tree items={items} renderItemTitle={titleRenderer} />

---

How we will get the same behaviour after this PR?

[kuzhelov]

@layershifter,

we are not losing anything with this change - for those components where this render functionality will be useful - here I am getting Tree case you've mentioned as an example, we will be able to use this value passed to renderItemTitle prop and pass it to the corresponding shorthand. Key difference is that we will have these render props only for those components' API where it is absolutely necessary (little percent of the whole number), and won't pollute the API surface for the others.

[mnajdova]

In this case, I find this methods useful. They are not callback render for a shorthand, but for each item of array. Do we want to get rid of this as well? It would be nice from user perspective for the array shorthands, to be able to define this logic via an API property. What are your thoughts on this @kuzhelov ?

[kuzhelov]

Solution with no additional API changes

// this could be a helper util
const customTreeShorthand = (value, renderTree) => (render => render(value, renderTree))

const renderCustomMenuItem = (Component, props) => <Component {...props} />

<Menu items={[ 
    { as: ..., ... },
    ...
  ]
 .map(shorthandValue => customTreeShorthand(shorthandValue, renderCustomMenuItem))
} 
.. />

Solution with render method introduced for limited set of components - specifically, collection and hierarchical ones: Menu, Tree, Accordion, List (?)

Needs changes, but we may talk about the final result even now:

<Menu items={[ 
    { as: ..., ... },
    ...
  ]}
  renderItem={(Component, props) => ... }  { /* is treated a default rendering logic, if not overwritten by shorthand */ }
.. />

What I don't like here is that it is not absolutely clear for the client where Component and props have come from, and where the data that is provided as shorthand object (i.e. { as: ..., ...} resides in those Component and props).

[mnajdova]

I am okay with both approaches, but when we have shorthand array, with two shorthands inside, as the panels are inside the Accordion, the user will have to define two maps for both shorthand, which may look awkward... Let's hear other folks' opinion before deciding.

[mnajdova]

👍 much cleaner API!

[mnajdova]

Same as the Accordion, I know that the user can use the current API and mapping logic for introducing this, but notice that this was not added with the renderXXX methods PR, it was here before, because it introduces clearer API for rendering each item in the Menu.

[mnajdova]

Let's decide whether we want to keep in the API the render method for the arrays of shorthands available in some components like Accordion, Menu, Tree etc. Other than that, the changes looks good!

[mnajdova]

👍

[mnajdova]

After offline discussion, we decided to add explicit render method just for the Tree component, the other ones can easily be defined by the user. Let's merge this after that is added.

[kuzhelov]

@mnajdova, this was already added, and I've not touched this method: https://github.com/stardust-ui/react/blob/master/src/components/Tree/Tree.tsx#L35 . It works as before :)