mui / material-ui

Material UI: Comprehensive React component library that implements Google's Material Design. Free forever.
https://mui.com/material-ui/
MIT License
93.87k stars 32.26k forks source link

[Select] Clicking a Select component when embedded within an iFrame causes the page to jump [Firefox] #27584

Open stevegreenn opened 3 years ago

stevegreenn commented 3 years ago

Current Behavior šŸ˜Æ

When an app using Material-UI's Select component is embedded within an IFrame, you get behaviour like this: alt text

As you can see when you click the Select input, the page jumps to the top of the IFrame

Expected Behavior šŸ¤”

The options menu should open without causing the page to jump as it does.

Steps to Reproduce šŸ•¹

App.js:

import * as React from 'react';
import MenuItem from '@material-ui/core/MenuItem';
import FormHelperText from '@material-ui/core/FormHelperText';
import FormControl from '@material-ui/core/FormControl';
import Select from '@material-ui/core/Select';

function App() {
  return (
      <div>
        <div style={{ height: '250px', backgroundColor: '#aaa'}}>{ /* empty space */ }</div>
        <div style={{ height: '250px', backgroundColor: '#bbb'}}>{ /* empty space */ }</div>
        <div style={{ height: '250px', backgroundColor: '#ccc'}}>{ /* empty space */ }</div>
        <FormControl sx={{ m: 1, minWidth: 120 }}>
          <Select>
            <MenuItem value="">
              <em>None</em>
            </MenuItem>
            <MenuItem value={10}>Ten</MenuItem>
            <MenuItem value={20}>Twenty</MenuItem>
            <MenuItem value={30}>Thirty</MenuItem>
          </Select>
          <FormHelperText>Without label</FormHelperText>
        </FormControl>
      </div>
  );
}

test.html:

<!doctype html>
<html lang="en">
    <body style="margin: 0;">
        <div style="height: 1000px;"></div>
        <iframe src="http://localhost:3000" style="border: 0; width: 100vw; height: 100vh;"></iframe>
        <div style="height: 1000px;"></div>
    </body>
</html>

Steps:

  1. Create an empty React project and use the above App.js
  2. Create the test.html using the provided code
  3. Start the server and open test.html in Firefox, at which point follow along with the above gif to see the page jumping.

Context šŸ”¦

We have our app embedded in an IFrame as part of a wider application and came across this page jumping issue.

Your Environment šŸŒŽ

`npx @material-ui/envinfo` ``` System: OS: Windows 10 10.0.18363 Binaries: Node: 14.17.3 - C:\Program Files\nodejs\node.EXE Yarn: 1.22.11 - ~\AppData\Roaming\npm\yarn.CMD npm: 6.14.13 - C:\Program Files\nodejs\npm.CMD Browsers: Firefox: 90.0.2 npmPackages: @emotion/react: ^11.4.0 => 11.4.0 @emotion/styled: ^11.3.0 => 11.3.0 @material-ui/core: ^5.0.0-beta.2 => 5.0.0-beta.2 @material-ui/private-theming: 5.0.0-beta.2 @material-ui/styled-engine: 5.0.0-beta.1 @material-ui/system: 5.0.0-beta.2 @material-ui/types: 6.0.1 @material-ui/unstyled: 5.0.0-alpha.41 @material-ui/utils: 5.0.0-beta.1 @types/react: 17.0.15 react: ^17.0.2 => 17.0.2 react-dom: ^17.0.2 => 17.0.2 ```
mnajdova commented 3 years ago

Can you please provide a CodeSandbox (https://material-ui.com/r/issue-template-next), or a link to a repository on GitHub that reproduces the problem?

stevegreenn commented 3 years ago

Here is a CodeSandbox: https://codesandbox.io/s/429zg Here is a direct link to the page containing an IFrame: https://429zg.csb.app/iframe.html

mnajdova commented 3 years ago

Alright, I can reproduce

eps1lon commented 3 years ago

Cannot reproduce on Firefox 88.0.1 nor 91.01 with a viewport of 320x1280 when scrolled to the bottom and opening the Select.

@mnajdova, @stevegreenn Can you still reproduce the problem? Could you include a screen recording using

stevegreenn commented 3 years ago

Hi @eps1lon, I can still reproduce the problem. Here is a link to a screen recording: https://streamable.com/10ohzw

I'm on Firefox 91 now.

ValeryP commented 2 years ago

Any idea how to solve this issue? I already thinking to give up on Material UI since it happens for all my forms.

StormRyders commented 2 years ago

Hi Team,

Any idea on how to solve this issue. This is a big blocker for us in using material-ui.

trismi commented 2 years ago

We are also experiencing this issue. A few notes, we are currently seeing this issue in both Chrome and Firefox. The top of the iFrame needs to be a bit scrolled off the top of the window in order to reproduce consistently. We've noticed this issue with both Select and Menu components.

ValeryP commented 2 years ago

Solved this issue by disabling auto-focus for all problematic view types: Select, DatePicker, etc.

<DatePicker
   {...{autoFocus: false}}
   minDate={minDate} 
   ...
</DatePicker>
StormRyders commented 2 years ago

<DatePicker {...{autoFocus: false}} minDate={minDate} ...

Hi Valery

Created one sandbox for the issue that we are seeing. Can you please look into that? https://429zg.csb.app/iframe.html

trismi commented 2 years ago

Solved this issue by disabling auto-focus for all problematic view types: Select, DatePicker, etc.

<DatePicker
   {...{autoFocus: false}}
   minDate={minDate} 
   ...
</DatePicker>

I am not able to get this working for me. Can you provide a demo where this solved the issue?

trismi commented 2 years ago

In some testing I've been doing, I think this may actually be an issue with Popover. To see what I mean, try this snippet of HTML:

<!doctype html>
<html lang="en">
    <body style="margin: 0;">
        <div style="height: 1000px;"></div>
        <iframe src="https://v4.mui.com/components/popover/#popover" style="border: 0; width: 100vw; height: 100vh;"></iframe>
        <div style="height: 1000px;"></div>
    </body>
</html>

When opening the Popover, the screen will jump.

Online link to example: https://erehd6.csb.app

michaldudak commented 2 years ago

Hi folks! I'm trying to find the root cause of the issue right now. I've noticed that the new implementation of Select - SelectUnstyled from the @mui/base package does not suffer from this issue (it also fixes a couple of other problems the Material UI version has). If you're willing to style it yourselves, it may be a viable option. Of course, I'll try to fix the Material UI Select as well.

michaldudak commented 2 years ago

I managed to improve things a little. Changes in #33437 (available to try using under https://pkg.csb.dev/mui/material-ui/commit/011091b0/@mui/material) seem to make the Popover work better, but the issue with Select still exists. It doesn't happen 100% of the time, though - it looks like a race condition somewhere. Still, with the fix I mentioned, the page doesn't scroll to the top of the iframe but it centers the Select in the viewport. I'll keep investigating it for some more time.

StormRyders commented 2 years ago

I managed to improve things a little. Changes in #33437 (available to try using under https://pkg.csb.dev/mui/material-ui/commit/011091b0/@mui/material) seem to make the Popover work better, but the issue with Select still exists. It doesn't happen 100% of the time, though - it looks like a race condition somewhere. Still, with the fix I mentioned, the page doesn't scroll to the top of the iframe but it centers the Select in the viewport. I'll keep investigating it for some more time.

We are using mui version @4.11.x and wanted to ensure that if the fix mentioned here will fix the issue in mui@4.11.x

trismi commented 2 years ago

I posted this question on Stack Overflow and received a helpful answer that does work. However, it requires that I then manage the focus state myself, and it'd be nice if that was not necessary.

miketierney commented 2 years ago

In the same vein as @trismi, we've disabled all of the autofocus in a project I'm working on (also on v4, but I'm hopeful that we can port a fix back to that even if it's not brought in to the older version of MUI), but we're now in a state where all keyboard-based input is broken.

I'll keep working to root cause this issue as well (as best we can on our end), but would love to see it resolved in the core library so we don't have to hack our fixes together.

michaldudak commented 2 years ago

@StormRyders as v4 is not in active development anymore, we focus only on providing fixes to the latest version.

guilhermeabell commented 2 years ago

I Managed to solve it as follows:

SelectProps={{ MenuProps:{ autoFocus: false, disableAutoFocusItem: true, disableEnforceFocus: true, disableAutoFocus: true, } }}

arfa123 commented 9 months ago

I Managed to solve it as follows:

SelectProps={{ MenuProps:{ autoFocus: false, disableAutoFocusItem: true, disableEnforceFocus: true, disableAutoFocus: true, } }}

While it successfully resolved the scrolling issue, it inadvertently introduced a new problem. Specifically, after implementing the suggested fix, the Select component does not lose focus when clicking outside of it; instead, it remains in focus. This behavior is not expected and needs to be addressed.

guilhermeabell commented 9 months ago

interesting. I didn't get this unexpected behavior, can you put your code in codesandbox?

guilhermeabell commented 9 months ago

@mnajdova

arfa123 commented 9 months ago

interesting. I didn't get this unexpected behavior, can you put your code in codesandbox?

Sure, here you see the effects after adding these props: https://codesandbox.io/p/sandbox/selectlabels-material-demo-forked-pj62hj chrome-capture-2024-1-4

mnajdova commented 9 months ago

Thanks for tagging me @guilhermeabell. @michaldudak was the last one who was looking into this, maybe he will have more info. If I remember correctly this was fixed in Base UI and we were looking into adopting Material UI to use the Base UI's hooks.

michaldudak commented 9 months ago

I was never able to get to the root of this problem. Since this will ultimately be fixed when we use Base UI's Select in Material UI, I didn't spend extra time on it.

arfa123 commented 8 months ago

Thanks for tagging me @guilhermeabell. @michaldudak was the last one who was looking into this, maybe he will have more info. If I remember correctly this was fixed in Base UI and we were looking into adopting Material UI to use the Base UI's hooks.

Could you kindly elaborate on how the hooks in Base UI were used to resolve this issue? Your guidance would be greatly appreciated.

michaldudak commented 8 months ago

They were not used to fix the problem specifically. We created a new implementation of Select in Base UI that happens to be free from this issue.

guilhermeabell commented 7 months ago

Hi @arfa123 what solved it for me in all the cases where I came across this problem was to disable autoFocus on the components that jumped SelectProps={{ MenuProps:{ autoFocus: false, disableAutoFocusItem: true, disableEnforceFocus: true, disableAutoFocus: true, } }} It's worth remembering that I was only able to reproduce/come across this problem using MUI when inside an application injected into iframes like iframe-resizer.

arfa123 commented 4 months ago

Hi @arfa123 what solved it for me in all the cases where I came across this problem was to disable autoFocus on the components that jumped SelectProps={{ MenuProps:{ autoFocus: false, disableAutoFocusItem: true, disableEnforceFocus: true, disableAutoFocus: true, } }} It's worth remembering that I was only able to reproduce/come across this problem using MUI when inside an application injected into iframes like iframe-resizer.

By doing this, the keyboard accessibility of the Select component does not function correctly.

arfa123 commented 4 months ago

They were not used to fix the problem specifically. We created a new implementation of Select in Base UI that happens to be free from this issue.

I am already using Material UI in my React app. Can I use Base UI along with Material UI together? If yes, how can I apply the same Material Design styles to a Base UI Select component?

michaldudak commented 4 months ago

We don't have a Material Design theme for Base UI, so you'll have to style your components on your own. I'd recommend browsing the Material UI's Select source code and taking style definitions from there.

arfa123 commented 3 months ago

my issue resolved by following this: https://stackoverflow.com/a/72913682/8785191