Issue with props merging on Menu

[layershifter]

Extracted from #1375.

Steps

1. Open codepen test case.
2. Click on an item of menu.

Expected Result

Call of handleItem and handleMenu on click on MenuItem.

Actual Result

Only handleItem was called.
In fact, we don't call handleItemClick when onClick was passed to an MenuItem.

Version

Latest.

Testcase

http://codepen.io/layershifter/pen/aJBmwz

[levithomason]

This is not a factory bug but the menu is incorrectly handling the items.

First, when creating shorthand items, the menu passes default props as an object. This means the items' onClick handler will override the default prop onClick handler:

    return _.map(items, (item, index) => MenuItem.create(item, {
      active: activeIndex === index,
      index,
      onClick: this.handleItemClick,
    }))

The menu should use the callback form of defaultProps which receives the user's props as an argument. This way, the onClick handler can wrap the user's onClick handler as well.

Second, the handleItemClick method assumes the items to be an array of objects from which it tries to lookup the clicked index and call the item's onClick handler. Instead, we should leave this to the wrapped item handler.

I'm not sure that we should auto wrap handlers in the factories as there may be cases where we want the override behavior.

[layershifter]

I'll make PR that updates tests and Menu items handling.

[levithomason]

I may have spoken prematurely on this as well. It will take more effort to diagnose properly. The defaultProps I believe are always overriden by the user's props. It is possible that even wrapping the user's props in the default props will be of no use as the user's handler will override the wrapped default handler:

https://github.com/Semantic-Org/Semantic-UI-React/blob/master/src/lib/factories.js#L46

const props = { ...defaultProps, ...usersProps }

Seems we need a way to define not defaultProps but some kind of overrideProps.

[layershifter]

It was fixed in #1428 ✌️