breaking(Portal): use createPortal() API #2755

layershifter · 2018-05-01T12:56:30Z

BREAKING

Why?

Our Portal component relies on unstable_renderSubtreeIntoContainer, this API is deprecated and will be removed in future releases of React. Also, this causes problems with tests.

1. Requirement of React 16

React 16 was released in September 2017, the update process is simple and I don't know any supported application that still uses React 15. It's time to say goodbye to React 15.

Please keep in mind, that in near future we will possibly require React 16.3.

2. Removal of `className` prop

As we use the new API (createPortal()) we don't need wrapper nodes as was before, so we don't have a target where we can apply className.

Before

<Portal className='foo'>
  <button>A button</button>
</Portal>

After

<Portal>
  <div className='foo'>
    <button>A button</button>
  </div>
</Portal>

3. Removal of `prepend` prop

Now, we don't control DOM manipulations, so this prop is useless now.

4. Removal of `closeOnRootNodeClick`

Now, we don't have rootNode at all, createPortal is responsible for DOM manipulations.

Fixes #2746.

layershifter · 2018-05-01T12:58:28Z

src/addons/Portal/PortalInner.js

+  render() {
+    const { children, mountNode = isBrowser() ? document.body : null } = this.props
+
+    return createPortal(<Ref innerRef={this.handleRef}>{children}</Ref>, mountNode)


We need a valid ref to get working closeOnDocumentClick as now we don't have rootNode and portalNode in fact

…ntic-UI-React into feat/portal-16

layershifter · 2018-05-03T17:19:20Z

docs/app/Examples/modules/Modal/Variations/ModalExampleCloseConfig.js

@@ -21,12 +21,10 @@ class ModalExampleCloseConfig extends Component {
        <Modal
          open={open}
          closeOnEscape={closeOnEscape}
-          closeOnRootNodeClick={closeOnRootNodeClick}
+          closeOnDimmerClick={closeOnDimmerClick}


closeOnRootNodeClick was not correct there

codecov-io · 2018-05-03T17:28:18Z

Codecov Report

Merging #2755 into react-16 will decrease coverage by <.01%.
The diff coverage is 100%.

@@             Coverage Diff              @@
##           react-16    #2755      +/-   ##
============================================
- Coverage     99.74%   99.74%   -0.01%     
============================================
  Files           160      161       +1     
  Lines          2762     2745      -17     
============================================
- Hits           2755     2738      -17     
  Misses            7        7

Impacted Files	Coverage Δ
src/addons/Portal/Portal.js	`100% <100%> (ø)`	⬆️
src/modules/Modal/Modal.js	`100% <100%> (ø)`	⬆️
src/addons/Portal/PortalInner.js	`100% <100%> (ø)`

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 6a22434...a996ad5. Read the comment docs.

levithomason · 2018-05-06T15:29:48Z

What do you think about making a react-16-3 branch for this? I know only 16.0 is required, but we can also bundle the ref changes into that branch. This way, we can gather all of our changes that require React >=16.3 and make one breaking release once it is all ready.

Linking #2747.

layershifter · 2018-05-08T15:31:49Z

I know only 16.0 is required, but we can also bundle the ref changes into that branch.

In this case I think it will be better if we ship this PR now. forwardRef is more complicated thing in fact.

levithomason · 2018-05-09T19:36:06Z

OK, let's merge this into a react-16 branch. I don't want to drop React 15 users on the next release.

…React into feat/portal-16 # Conflicts: # src/index.js

layershifter · 2018-05-13T13:14:08Z

@levithomason I've changed base to react-16 branch and merged.

* breaking(Portal): use createPortal() API (#2755) * breaking(Portal): use createPortal() API * test(Portal): rewrite tests * test(Portal): rewrite tests * fix(Portal): mixed fixes in tests * fix(Portal): fix path of typings * feat(Flag|Icon): use PureComponent instead of shallowEqual() (#2842) * fix(IconSearch): remove prop * fix(PortalInner): remove meta * fix(Portal): fix broken test * fix(Modal): go away from rootNode * fix(Flag|Icon): remove unused import * feat(Portal): add `triggerRef` prop (#2902) * feat(Portal): add `triggerRef` prop * test(Portal): add test for `triggerRef` prop * fix(Popup): fix test * BREAKING(Accordion): refactor shorthand API to use AccordionPanel (#2904) * refactor(Accordion): simplify render with AccordionPanel * style(Accordion): add description * fix(tests): reenable throw on console.log() * style(mixed): lint issues in typings * fix yarn.lock * fix(ComponentDocLinks): add rel="noopener noreferrer" * revert styling changes * restore yarn.lock from master * style(Portal): remove render method

breaking(Portal): use createPortal() API

1ffd38f

layershifter added the enhancement label May 1, 2018

layershifter commented May 1, 2018

View reviewed changes

layershifter added 5 commits May 2, 2018 22:07

test(Portal): rewrite tests

a2d00c2

test(Portal): rewrite tests

e3fff3a

Merge branch 'master' into feat/portal-16

4071961

fix(Portal): mixed fixes in tests

7d71765

Merge branch 'feat/portal-16' of https://github.com/Semantic-Org/Sema…

5b8ac91

…ntic-UI-React into feat/portal-16

layershifter commented May 3, 2018

View reviewed changes

fix(Portal): fix path of typings

5b7de82

layershifter added the ready for review label May 3, 2018

layershifter requested a review from levithomason May 3, 2018 17:32

layershifter changed the base branch from master to react-16 May 13, 2018 13:01

Merge branch 'master' of https://github.com/Semantic-Org/Semantic-UI-…

a996ad5

…React into feat/portal-16 # Conflicts: # src/index.js

layershifter merged commit c33a720 into react-16 May 13, 2018

layershifter deleted the feat/portal-16 branch May 13, 2018 13:14

layershifter mentioned this pull request May 19, 2018

Update Portal to use ReactDOM.createPortal #2746

Closed

layershifter mentioned this pull request Jun 12, 2018

BREAKING(package): update to React 16 #2880

Merged

levithomason added the breaking-change label Dec 10, 2018

layershifter mentioned this pull request Mar 18, 2019

Dropdown: child's onclick firing dropdown's onclick #3517

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

breaking(Portal): use createPortal() API #2755

breaking(Portal): use createPortal() API #2755

layershifter commented May 1, 2018 •

edited

Loading

layershifter May 1, 2018

layershifter May 3, 2018

codecov-io commented May 3, 2018 •

edited

Loading

levithomason commented May 6, 2018

layershifter commented May 8, 2018

levithomason commented May 9, 2018

layershifter commented May 13, 2018

breaking(Portal): use createPortal() API #2755

breaking(Portal): use createPortal() API #2755

Conversation

layershifter commented May 1, 2018 • edited Loading

BREAKING

Why?

1. Requirement of React 16

2. Removal of className prop

Before

After

3. Removal of prepend prop

4. Removal of closeOnRootNodeClick

layershifter May 1, 2018

Choose a reason for hiding this comment

layershifter May 3, 2018

Choose a reason for hiding this comment

codecov-io commented May 3, 2018 • edited Loading

Codecov Report

levithomason commented May 6, 2018

layershifter commented May 8, 2018

levithomason commented May 9, 2018

layershifter commented May 13, 2018

layershifter commented May 1, 2018 •

edited

Loading

2. Removal of `className` prop

3. Removal of `prepend` prop

4. Removal of `closeOnRootNodeClick`

codecov-io commented May 3, 2018 •

edited

Loading