Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Tooltip] create around RootRef #14574

Closed
2 tasks done
jonfreedman opened this issue Feb 18, 2019 · 6 comments
Closed
2 tasks done

[Tooltip] create around RootRef #14574

jonfreedman opened this issue Feb 18, 2019 · 6 comments
Labels
component: tooltip This is the name of the generic UI component, not the React module! discussion

Comments

@jonfreedman
Copy link

  • This is not a v0.x issue.
  • I have searched the issues of this repository and believe that this is not a duplicate.

Expected Behavior 馃

Tooltip should be functional and inner or ref exposed

Current Behavior 馃槸

Warning on the console and tooltip is not functional although ref is exposed

Steps to Reproduce 馃暪

Link: https://codesandbox.io/s/2wy34rmpj0

  1. See warning message in console

Context 馃敠

I want to display a tooltip for a table cell and also access the underlying dom element of the cell

Your Environment 馃寧

Tech Version
Material-UI v3.9.2
React 16.8.1
Browser Chrome 71
TypeScript 3.3.1
@oliviertassinari
Copy link
Member

oliviertassinari commented Feb 18, 2019

@jonfreedman What about the warning in the console? Moving the Tooltip component one level up or down solve the problem. Can you confirm?

@MalignantShadow
Copy link
Contributor

If you wrap RootRef around the Tooltip the ref will point to the TableCell (th) node. (Changed the component in the example to a class to log ref)

class SimpleTable extends React.Component {

  ref = React.createRef()

  render() {
    const { classes } = this.props;
    return (
      <Paper className={classes.root}>
        <Table className={classes.table}>
          <TableHead>
            <TableRow>
              <TableCell>Dessert (100g serving)</TableCell>
              <RootRef rootRef={this.ref}>
                <Tooltip title="Cal" placement="right">
                  <TableCell align="right">Calories</TableCell>
                </Tooltip>
              </RootRef>
              <TableCell align="right">Fat (g)</TableCell>
              <TableCell align="right">Carbs (g)</TableCell>
              <TableCell align="right">Protein (g)</TableCell>
            </TableRow>
          </TableHead>
          <TableBody>
            {rows.map(row => (
              <TableRow key={row.id}>
                <TableCell component="th" scope="row">
                  {row.name}
                </TableCell>
                <TableCell align="right">{row.calories}</TableCell>
                <TableCell align="right">{row.fat}</TableCell>
                <TableCell align="right">{row.carbs}</TableCell>
                <TableCell align="right">{row.protein}</TableCell>
              </TableRow>
            ))}
          </TableBody>
        </Table>
      </Paper>
    );
  }

  componentDidMount() {
    console.log(this.ref.current)
  }

}

Solves the problem, logs

<th class="MuiTableCell-root-1195 MuiTableCell-head-1196 MuiTableCell-alignRight-1205" scope="col" title="Cal">Calories</th>

to console. But is that confusing?

<RootRef ref={ref}>
  <Tooltip title="Some title">{content}</Tooltip>
</RootRef>

Wouldn't this imply we want to reference the root node of the Tooltip, or in this case, the Popper? (Even if that isn't the root component)

@oliviertassinari
Copy link
Member

oliviertassinari commented Feb 19, 2019

I wouldn't fight too much with the RootRef component. We will remove it once we have a viable alternative. React has deprecated the findDOMNode() API that the component relies on.

The tooltip renders a fragment, the first child is the wrapped children. findDOMNode always returns the first child, so it works.

In the future, you will be able to attach a ref directly to the TableCell or use the React.Fragment ref component. cc @eps1lon

@oliviertassinari oliviertassinari added discussion component: tooltip This is the name of the generic UI component, not the React module! and removed waiting for user information labels Feb 19, 2019
@jonfreedman
Copy link
Author

@oliviertassinari I look forward to using innerRef directly - I couldn't get it to work on the latest version (React was telling me I should use a ForwardRef). I did fix the issue by putting the Tooltip around the cell text and the RootRef around the TableCell

@oliviertassinari
Copy link
Member

oliviertassinari commented Feb 19, 2019

@jonfreedman I was referring to the usage of the ref property directly. Oh and yes, you can always use a ref + findDOMNode.

@eps1lon
Copy link
Member

eps1lon commented Feb 19, 2019

@jonfreedman The inner component of TableCell is currently a function component which is why you can't use innerRef. We recognized this issue and are in the process of fixing this. Unfortunately there aren't alot of battle tested patterns for using refs instead of findDOMNode. We can't "just" use refs instead of findDOMNode.

You may be interested in subscribing to #14415. We're tracking progress there about the general state of refs in material-ui. We hope to support strict mode in v4 while keeping the migration cost for non-strict mode users as low as possible.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: tooltip This is the name of the generic UI component, not the React module! discussion
Projects
None yet
Development

No branches or pull requests

4 participants