Semantic-Org / Semantic-UI-React

The official Semantic-UI-React integration
https://react.semantic-ui.com
MIT License
13.21k stars 4.05k forks source link

node.contains is not a function in Portal click #3786

Closed xumix closed 5 years ago

xumix commented 5 years ago

Bug Report

Can't actually create a valid bug report, looks like passed node is not a real DOM node. The issue appeared after this PR I suppose: https://github.com/Semantic-Org/Semantic-UI-React/pull/2384 Here is a similar issue in react dropzone https://github.com/react-dropzone/react-dropzone/issues/763 and here https://stackoverflow.com/questions/50133673/this-node-contains-does-not-work-if-this-node-is-a-ref-to-react-component

Expected Result

Clicks work as intended

Actual Result

No click events are fired

Node here is:

node: {…}
​_reactInternalFiber: {…}
​_reactInternalInstance: {…}
​computeElementType:Button/<()
​computeTabIndex:Button/<()
​context: {}
​focus:Button/<()
​handleClick:Button/<()
​hasIconClass:Button/<()
​props: {…}
​ref: {…}
​refs: {}
​state: null
​updater: {…}
​<prototype>: {…}
TypeError: node.contains is not a functiondoesNodeContainClick.js:25
    doesNodeContainClick doesNodeContainClick.js:25
    Portal Portal.js:59
    dispatchEvent event-stack.development.js:162
    dispatchEvent event-stack.development.js:284
    EventTarget event-stack.development.js:340
    forEach self-hosted:3739
    EventTarget event-stack.development.js:339

Version

0.88.1

layershifter commented 5 years ago

Please provide a repro case.

xumix commented 5 years ago

@layershifter any chance for investigation w/o repro? I've tried to isolate it, but have too many deps in this modal :(

layershifter commented 5 years ago

How? 🤔

As I see it because wrong ref to trigger/content. I also can be that SUIR Button is used and a ref to it should be captured.

There is only one obvious case than can be an issue:

const CustomButton = React.forwardRef((props, ref) => {
  return <Button {...props} ref={ref} />
})

Try to simplify your Modal by maximum and provide a clean repro. Then I can provide a solution/fix.

xumix commented 5 years ago

Yup. looks like it's StyledComponent button as trigger that is the cause

import { Button } from "semantic-ui-react";
import styled from "styled-components/macro";

import semanticStyles from "../../../semantic/exports.module.less";

export const LinkButton = styled(Button).attrs({
  //className: "tertiary"
})`
  color: ${semanticStyles.blue} !important;
  font-weight: normal !important;
  padding: 0 !important;
  background-color: transparent !important;
`;
<React.Fragment>
        <Modal
          centered={false}
          trigger={<LinkButton onClick={this.handleOpen}>{this.props.elementText}</LinkButton>}
          dimmer={"inverted"}
          open={this.state.modalOpen}
          onClose={this.handleClose}
          size={"large"}>
          <Modal.Header>{r.offerName}</Modal.Header>
          <Modal.Content scrolling>
            <OfferSkuMatchComponent
              isAdmin={isAdmin}
            />
          </Modal.Content>
          <Modal.Actions>
            <Button primary onClick={this.handleClose}>
              Закрыть
            </Button>
          </Modal.Actions>
        </Modal>
      </React.Fragment>
xumix commented 5 years ago

Now, looks like it's Styled design decision, I just don't get how do I forward ref to styled button https://www.styled-components.com/docs/advanced#refs They pass ref to component instance instead of DOM node, so here is the problem I suppose

layershifter commented 5 years ago

Let me create a repro and ensure 👍

layershifter commented 5 years ago

Here is a workaround for you. Can you please check?

const ButtonWithRef = React.forwardRef((props, ref) => (
  <Ref innerRef={ref}>
    <Button {...props} />
  </Ref>
));

export const WorkingButton = styled(ButtonWithRef)`
  color: green !important;
`;

https://codesandbox.io/s/semantic-ui-example-oxqmt

xumix commented 5 years ago

@layershifter Thank you for your efforts! It worked!

xumix commented 5 years ago

Does this mean that now I must use this workaround every time I use StyledComponents and Modals?

layershifter commented 5 years ago

Yep and Popups, too. As in your case ref will be forwarded to the class component. I know about this issue, but I don't how it can be improved right now.

xumix commented 5 years ago

Well, this is really unfortunate :( Also now I'm getting this in console:

Warning: Failed prop type: The prop `innerRef` is marked as required in `Ref`, but its value is `null`.
    in Ref (at LinkButtonStyles.tsx:8)
    in ForwardRef (created by Context.Consumer)
    in StyledComponent (created by LinkButtonStyles__LinkButton)

when I use the button without trigger and modal

layershifter commented 5 years ago

Can you please edit CodeSandbox to show your issue?

xumix commented 5 years ago

@layershifter I've managed to make it work like this:

const ButtonWithRef = (React.forwardRef((props, ref) =>
  ref ? (
    <Ref innerRef={ref}>
      <Button {...props} />
    </Ref>
  ) : (
    <Button {...props} />
  )
) as any) as typeof Button;
layershifter commented 5 years ago

Good to know, closing for now.

bionicvapourboy commented 4 years ago

In my case (Styled Component issue) this simple trick has done the job:

const RefButton = props => <StyledButton {...props} />;

zgrybus commented 3 years ago

Any updates? Would be nice, if you could fix this issue.

loucadufault commented 6 months ago

TypeScript:

import React from 'react'
import { Button, Ref } from 'semantic-ui-react'

const ButtonWithRef = React.forwardRef<HTMLElement>((props, ref) => (
  <Ref innerRef={ref}>
    <Button {...props} />
  </Ref>
)) as any as typeof Button