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

[transitions] "Cannot read property scrollTop of null" #27154

Open
3 tasks done
siriwatknp opened this issue Jul 7, 2021 · 16 comments
Open
3 tasks done

[transitions] "Cannot read property scrollTop of null" #27154

siriwatknp opened this issue Jul 7, 2021 · 16 comments
Assignees
Labels
component: transitions This is the name of the generic UI component, not the React module! docs Improvements or additions to the documentation v5.x migration waiting for 馃憤 Waiting for upvotes

Comments

@siriwatknp
Copy link
Member

siriwatknp commented Jul 7, 2021

  • The issue is present in the latest release.
  • I have searched the issues of this repository and believe that this is not a duplicate.

Current Behavior 馃槸

If the child of components does not forwardRef to DOM, it cause this error.

Screen Shot 2564-07-07 at 09 24 14

Expected Behavior 馃

The error should be more informative about how to fix it.

Steps to Reproduce 馃暪

Steps:

  1. open this sandbox https://codesandbox.io/s/transition-v5-error-dide8?file=/src/App.tsx
  2. change material-ui/core to v5.beta
  3. will see the error

Context 馃敠

Is it better to display warning (in console) instead of Error?

Here is what I think about the dev experience.

  • write the code (use Transition the wrong way, no forwardRef for child)
  • check in browser, it does not work (the ui display without transitioning)

then, 3 possibilities

  • they see the warning in console about why it does not work and link to full doc example
  • they go to Transition docs
  • worse case, they open the issue

Actions

Your Environment 馃寧

`npx @material-ui/envinfo`
  Don't forget to mention which browser you used.
  Output from `npx @material-ui/envinfo` goes here.
@siriwatknp siriwatknp added status: waiting for maintainer These issues haven't been looked at yet by a maintainer component: transitions This is the name of the generic UI component, not the React module! and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Jul 7, 2021
@thovden
Copy link

thovden commented Jul 7, 2021

Thanks - this saved me some debugging. I had this construct:

<Zoom in timeout={200}>
  <React.Fragment>

Fixed by adding the actual components as the only child, e.g.,

<Zoom in timeout={200}>
  <Fab>

@oliviertassinari
Copy link
Member

I'm linking #17119 too for context when a similar symptom impacts the tests (but with a different root cause).

With v5, the first two warnings in the console are about how to fix the issue https://codesandbox.io/s/transition-v5-error-forked-x66jg

Capture d鈥檈虂cran 2021-07-14 a虁 18 31 14

and then, about the exception that is thrown. The problem is very close to #18119 and #17480.

@oliviertassinari oliviertassinari changed the title [Transition] Cannot read property scrollTop of null [transitions] Cannot read property scrollTop of null Jul 14, 2021
@oliviertassinari oliviertassinari changed the title [transitions] Cannot read property scrollTop of null [transitions] "Cannot read property scrollTop of null" Jul 14, 2021
@oliviertassinari oliviertassinari added the waiting for 馃憤 Waiting for upvotes label Jul 15, 2021
@codingedgar
Copy link

I'm checking the example at https://mui.com/pt/guides/migration-v4/#cannot-read-property-scrolltop-of-null that linked here, but I get this error while using styled and wrapping it in a forwardRef (like this DefinitelyTyped/DefinitelyTyped#28884 (comment)) does not solve it.

I have a while trying to find and tinkering how to do this right but honestly haven't found anything, do you have an example of how to use Fade with styled component as a child?

@siriwatknp
Copy link
Member Author

I have a while trying to find and tinkering how to do this right but honestly haven't found anything, do you have an example of how to use Fade with styled component as a child?

Please provide a CodeSandbox that describes your issue.

@codingedgar
Copy link

codingedgar commented Mar 21, 2022

https://codesandbox.io/s/simplefade-material-demo-forked-9gm3tp?file=/demo.tsx:0-2357

just in case here is the code:

import React from "react";
import {
  InputAdornment,
  Input,
  InputProps,
  IconButton,
  styled
} from "@mui/material";
import { Visibility, VisibilityOff } from "@mui/icons-material";

export type PasswordInputProps = InputProps & {
  showPassword: boolean;
  onToggleShowPassword: (
    event: React.MouseEvent<HTMLButtonElement, MouseEvent>
  ) => void;
};

export const PasswordInput = styled(
  ({ showPassword, onToggleShowPassword, ...props }: PasswordInputProps) => (
    <Input
      placeholder="Link Password"
      type={showPassword ? "test" : "password"}
      endAdornment={
        // eslint-disable-next-line react/jsx-wrap-multilines
        <InputAdornment position="end">
          <IconButton
            aria-label="toggle password visibility"
            edge="end"
            onMouseDown={(e) => {
              e.preventDefault();
            }}
            onClick={onToggleShowPassword}
          >
            {showPassword ? (
              <VisibilityOff fontSize="small" />
            ) : (
              <Visibility fontSize="small" />
            )}
          </IconButton>
        </InputAdornment>
      }
      {...props}
    />
  )
)({
  backgroundColor: "var(--color-3)",
  color: "var(--color-1)",
  fontSize: 12,
  borderRadius: "5px",
  paddingLeft: "10px",
  alignItems: "center",
  "&:focus": {
    backgroundColor: "var(--color-3)"
  },
  width: "250px",
  height: "30px",
  "&:after": {
    borderColor: "var(--color-3)"
  },
  "&.MuiInput-underline:hover:not(.Mui-disabled):before": {
    borderBottom: "none"
  },
  "&.MuiInput-underline:before": {
    borderBottom: "none"
  },
  "& .MuiInput-input": {
    "&[type=password]:not(:placeholder-shown)": {
      fontSize: 17,
      letterSpacing: -2,
      fontFamily: "monospace",
      padding: 0
    }
  },
  "& .MuiInputAdornment-root": {
    color: "var(--color-1)",
    margin: 0,
    padding: "0 10px",
    backgroundColor: "var(--color-3)",
    "& button": {
      color: "var(--color-1)"
    }
  }
});

// from: https://github.com/DefinitelyTyped/DefinitelyTyped/issues/28884#issuecomment-983243411
// the only diff is that i do not use <Omit> as forwardRef already does that
export const PasswordInputRef = React.forwardRef<
  typeof Input,
  PasswordInputProps
>((props, ref) => <PasswordInput ref={ref} {...props} />);
import * as React from "react";
import Box from "@mui/material/Box";
import Switch from "@mui/material/Switch";
import Fade from "@mui/material/Fade";
import FormControlLabel from "@mui/material/FormControlLabel";
import { PasswordInputRef, PasswordInput } from "./PasswordInput";
import Input from "@mui/material/Input";

export default function SimpleFade() {
  const [checked0, setChecked0] = React.useState(false);
  const [checked1, setChecked1] = React.useState(false);
  const [checked2, setChecked2] = React.useState(false);
  const [checked3, setChecked3] = React.useState(false);
  const [showPassword, toggleShowPassword] = React.useState(false);

  const handleChange0 = () => {
    setChecked0((prev) => !prev);
  };

  const handleChange1 = () => {
    setChecked1((prev) => !prev);
  };

  const handleChange2 = () => {
    setChecked2((prev) => !prev);
  };

  const handleChange3 = () => {
    setChecked3((prev) => !prev);
  };

  return (
    <Box sx={{ height: 180 }}>
      <FormControlLabel
        control={<Switch checked={checked0} onChange={handleChange0} />}
        label="Show0"
      />
      <FormControlLabel
        control={<Switch checked={checked1} onChange={handleChange1} />}
        label="Show1"
      />
      <FormControlLabel
        control={<Switch checked={checked2} onChange={handleChange2} />}
        label="Show2"
      />
      <FormControlLabel
        control={<Switch checked={checked3} onChange={handleChange3} />}
        label="Show3"
      />
      <Box sx={{ display: "flex" }}>
        <Fade in={checked0}>
          <Input />
        </Fade>
        <Fade in={checked1}>
          <PasswordInputRef
            showPassword={showPassword}
            onToggleShowPassword={() => {
              toggleShowPassword((x) => !x);
            }}
          />
        </Fade>
        <Fade in={checked2}>
          <PasswordInput
            showPassword={showPassword}
            onToggleShowPassword={() => {
              toggleShowPassword((x) => !x);
            }}
          />
        </Fade>
        <Fade in={checked3}>
          <div>
            <PasswordInput
              showPassword={showPassword}
              onToggleShowPassword={() => {
                toggleShowPassword((x) => !x);
              }}
            />
          </div>
        </Fade>
      </Box>
    </Box>
  );
}

which returns

TypeError
Cannot read properties of null (reading 'scrollTop')

for check1 and check2, my workaround has been check3 but I wish to know why or how styled does not forward the ref to Input, or if I'm seeing this wrong? as check0 which is the Input without styled wrap, does work fine.

@siriwatknp
Copy link
Member Author

@codingedgar I think the problem is in the PasswordInput. You have to forward the ref to the Input.

export const PasswordInput = styled(
  React.forwardRef(({ showPassword, onToggleShowPassword, ...props }: PasswordInputProps, ref) => (
    <Input
      ref={ref}
      placeholder="Link Password"
      type={showPassword ? "test" : "password"}
      endAdornment={
        // eslint-disable-next-line react/jsx-wrap-multilines
        <InputAdornment position="end">
          <IconButton
            aria-label="toggle password visibility"
            edge="end"
            onMouseDown={(e) => {
              e.preventDefault();
            }}
            onClick={onToggleShowPassword}
          >
            {showPassword ? (
              <VisibilityOff fontSize="small" />
            ) : (
              <Visibility fontSize="small" />
            )}
          </IconButton>
        </InputAdornment>
      }
      {...props}
    />
  ))
)

@codingedgar
Copy link

@codingedgar I think the problem is in the PasswordInput. You have to forward the ref to the Input.

export const PasswordInput = styled(
  React.forwardRef(({ showPassword, onToggleShowPassword, ...props }: PasswordInputProps, ref) => (
    <Input
      ref={ref}
      placeholder="Link Password"
      type={showPassword ? "test" : "password"}
      endAdornment={
        // eslint-disable-next-line react/jsx-wrap-multilines
        <InputAdornment position="end">
          <IconButton
            aria-label="toggle password visibility"
            edge="end"
            onMouseDown={(e) => {
              e.preventDefault();
            }}
            onClick={onToggleShowPassword}
          >
            {showPassword ? (
              <VisibilityOff fontSize="small" />
            ) : (
              <Visibility fontSize="small" />
            )}
          </IconButton>
        </InputAdornment>
      }
      {...props}
    />
  ))
)

Thank you so much! It does work, was unsure on how to use styled and forwardRef together.

@totszwai
Copy link

totszwai commented May 4, 2022

Also facing the same problem... so are you saying that if the child is a styled-component and does not have a forwardRef it would not work...? Shouldn't the Fade (or all transitions) work with or without a ref?

We have many components that are just plain styled-components without a ref (because there were simply no need to use a ref) but now we have to add it everywhere just to get transition to work??

Perhaps all transitions should wraps the child with a plain div instead?

The workaround I had to do, was to manually wrap the children with another div... 馃憥

      <Fade in={visible} timeout={1000}>
        <div>{children}</div>
      </Fade>

@chuckwired
Copy link

Thanks @totszwai ! I can confirm I'm experiencing the same on <Fade> upgrading my codebase from v4 to v5, and wrapping with a div circumvents the issue.

@katherinedavenia
Copy link

Thanks @totszwai . I faced the same problem and your solution works. I need to wrap the children with a div tag.

@relativeflux
Copy link

relativeflux commented Jul 7, 2022

@totszwai Can confirm this solution of wrapping in a div works. Can you please look into this MUI team? We shouldn't have to resort to this workaround.

@sin-to-jin
Copy link

sin-to-jin commented Jul 20, 2022

It might be to differ a behavior, but I tried to take to use <Collapse> instead of <Fade>, then it had no error.

  • "react": "18.1.0",
  • "@mui/material": "^5.7.0",
// TypeError: Cannot read properties of null (reading 'scrollTop').
import { Fade } from '@mui/material';
const Component = () => {
  const [checked, setChecked] = React.useState(false);
  return (
    <>
      <button onClick={() => setChecked(!checked)}>test</button>
      <Fade in={checked}>
        <div>hoge</div>
      </Fade>
    </>
  );
}
// no errors
import { Collapse } from '@mui/material';
const Component = () => {
  const [checked, setChecked] = React.useState(false);
  return (
    <>
      <button onClick={() => setChecked(!checked)}>test</button>
      <Collapse in={checked}>
        <div>hoge</div>
      </Collapse>
    </>
  );
}

@bendras
Copy link

bendras commented Oct 18, 2022

Observed similar issue in Tabs /> component. SEE: #17119 (comment)

@allicanseenow
Copy link

allicanseenow commented Feb 13, 2023

This issue is still existing and was not available with MUI 4. This can cause the a project with hundreds of components to crash after the upgrade to MUI 5 and it's difficult to fix because the stack trace or the logged error message is not helpful at all as it doesn't mention where or which MUI component is causing the issue.

@dberardo-com
Copy link

yeah so ... how to use transitions on mounting / rerendering components with MUI5 then ? any solution? any workaround ? or is this only affecting Fade ?

cfl777 added a commit to cfl777/osapiens-challenge that referenced this issue Sep 5, 2023
Due to multiple issues that occur on the MUI transition in this instance, the grow transition is disabled.
This isn't the optimal solution, further investigation is needed.

Attempted to apply the solutions mentioned on bug reports. Added a React.Fragment wrap, which resolved only some of the issues. Specifically the "Uncaught TypeError: Cannot read properties of null (reading 'scrollTop').

Might be missing something small. See
mui/material-ui#27154
mui/material-ui#18119
mui/material-ui#17119
cfl777 added a commit to cfl777/osapiens-challenge that referenced this issue Sep 5, 2023
Due to multiple issues that occur on the MUI transition in this instance, the grow transition is disabled.
This isn't the optimal solution, further investigation is needed.

Attempted to apply the solutions mentioned on bug reports. Added a React.Fragment wrap, which resolved only some of the issues. Specifically the "Uncaught TypeError: Cannot read properties of null (reading 'scrollTop').

Might be missing something small. See
mui/material-ui#27154
mui/material-ui#18119
mui/material-ui#17119
@shughes-uk
Copy link

Had the same issue, for me I was using forwardRef wrong with a custom Card

Here's what I was doing wrong (pseduocode)

// INCORRECT
forwardRef(({ ref, ...props }) => <Card ref={ref} ...props>blah</Card>
// CORRECT
forwardRef((props, ref) => <Card ref={ref} ...props>blah</Card>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: transitions This is the name of the generic UI component, not the React module! docs Improvements or additions to the documentation v5.x migration waiting for 馃憤 Waiting for upvotes
Projects
None yet
Development

Successfully merging a pull request may close this issue.