docs(Modal): Add controlled modal example

[lewisblackwood]

An example of a controlled modal component utilising the open prop. Fixes #690.

@levithomason could you advise on how to include open in the prop table at the top of the document?

[codecov-io]

Current coverage is 100% (diff: 100%)

No coverage report found for master at 11310c3.

Powered by Codecov. Last update 11310c3...e79fa81

[levithomason]

You can read about docs in the contributing guide:

https://github.com/Semantic-Org/Semantic-UI-React/blob/master/.github/CONTRIBUTING.md#documentation

[levithomason]

Class name should match the file name.

[levithomason]

Let me know if you have any other questions.

[lewisblackwood]

I've added the reference for the open prop to Modal.js and docgenInfo.json, so the open prop is now included when the docs get built.

It looks like this has broken some of the functionality of the Modal component, as the changes to the open prop don't get always get passed through to the Portal's state. For example, the controlled component I added now closes on a click outside the portal, but not when handleClose is called from the button.

@levithomason does the Modal component need a lifecycle method similar to that in the Dropdown component? Apologies if I've missed something simple here!

[levithomason]

Previously the open prop was captured and spread in the unhandled variable. This happens in the render() with the getUnhandledProps() util.

Now that the prop is being "handled" (added to the propTypes), you'll need to destructure it from props and pass it manually. It is no longer captured in the unhandled variable. Add it here, then pass it to the Portal: <Portal open={open} />.

Clarification, docgenInfo.json is not committed to the repo. It is auto generated from the propTypes, so just adding it to propTypes is enough. Your changes to docgenInfo.json won't actually push to GitHub. That file is ignored, it is also deleted and recreated locally on every build 👍

[lewisblackwood]

Thanks for the clarification @levithomason!

The controlled component now works 👍

What's the best way to maintain the functionality in the uncontrolled components, where open isn't passed as a prop?

I've been looking at the pattern of activeIndex in Accordion, but I'm not sure how to apply it to the modal. Should Modal now inherit from AutoControlledComponent and have open set as as autoControlledProps?

[jeffcarbs]

@lewisblackwood - previously the modal was passing open through via getUnhandledProps. It's now passing it through explicitly, however the result is still the same. The Portal is the AutoControlledComponent so it will handle keeping track of open unless it receives a defined value for it. If open wasn't passed when using the Modal it will be passed as undefined to Portal which means that Portal will control the open state.

If the tests are all passing and the examples still work on this branch, you should be g2g. If not, maybe I'm misunderstanding how AutoControlledComponent works.

[lewisblackwood]

Sorry @jcarbo, I should have been clearer above: I'm finding that the uncontrolled modal examples in the docs don't open on this branch (despite the passing tests). The controlled examples (mine and those under the 'Variations' header) are working.

[jeffcarbs]

@lewisblackwood - indeed my understanding of AutoControlledComponent was wrong. I opened a PR with a potential fix and further discussion on how to handle this type of behavior: #788

[levithomason]

@lewisblackwood there have been some updates to the Modal, you might want merge or rebase to the latest master to keep this PR up to speed. It looks like there are some conflicts.

[lewisblackwood]

@levithomason thanks for the heads-up; I've rebased this PR. Just waiting to see the outcome of #788 as this PR still breaks the uncontrolled modal right now.

[levithomason]

@lewisblackwood this PR should be unblocked and ready to update to the latest master. Give it a whirl and LMK how it goes.

Details in #788.

[lewisblackwood]

@levithomason rad! All the modal examples are working now, this should be good to merge 👍

[levithomason]

Fixed a typo in the prop description, will ship on pass. Thanks much 🎉