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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Wrong positioning of popover on IOS #3638

Closed
w01fgang opened this issue Mar 8, 2016 · 34 comments
Closed

Wrong positioning of popover on IOS #3638

w01fgang opened this issue Mar 8, 2016 · 34 comments
Labels
bug 🐛 Something doesn't work component: autocomplete This is the name of the generic UI component, not the React module! component: Popover The React component.

Comments

@w01fgang
Copy link

w01fgang commented Mar 8, 2016

Problem Description

Popover doesn't seen on IOS when I type in Autocomplete field.

video

I guess I found why.

video
When I start to interact with other parts of application, the popover flies from top.
I found that div with popover has wrong position. For example: the text field has absolute offset position 916px and popover has top: 294px;.
I think that in IOS position:fixed has different behavior, positioning starts from top of the page, not of the screen.

Versions

  • Material-UI: v0.15.0-alpha.1 and latest from github
  • React: 0.14.7
  • Browser: Safari 8 / ios 8.2
@nathanmarks nathanmarks added the bug 🐛 Something doesn't work label Mar 9, 2016
@maxpechenin
Copy link
Contributor

I was facing this problem on one of my projects. http://stackoverflow.com/questions/7970389/ios-5-fixed-positioning-and-virtual-keyboard . Shortly: fixed elements scrolls in iOS browsers, then virtual keyboard is opened. Moreover, devs from Apple keep this behaviour at least in a 5 iOS versions in a row. So probably it's a feature and they aren't going to fix it. I can envestigate, how to solve this in popover, if you want.

@w01fgang
Copy link
Author

w01fgang commented Mar 9, 2016

To override this behavior I check if it's IOS and if true I get offset().top instead of anchor element bottom position.
I use jquery and device.js because deadline

getAnchorPosition(el) {
    if (!el) {
      el = ReactDOM.findDOMNode(this);
    }
    const rect = el.getBoundingClientRect();
    const a = {
      top: rect.top,
      left: rect.left,
      width: el.offsetWidth,
      height: el.offsetHeight,
    };
    const offsetTop = window.jQuery(el).offset().top;
    a.right = rect.right || a.left + a.width;
    a.bottom = (device.ios()?offsetTop + a.height:rect.bottom) || (device.ios()?offsetTop:a.top) + a.height;
    a.middle = a.left + ((a.right - a.left) / 2);
    a.center = a.top + ((a.bottom - a.top) / 2);
    return a;
  },

@w01fgang w01fgang closed this as completed Mar 9, 2016
@w01fgang w01fgang reopened this Mar 9, 2016
@w01fgang
Copy link
Author

w01fgang commented Mar 9, 2016

Sorry, missed by clicking on the button :)

@maxpechenin
Copy link
Contributor

Seems like we can safely change this https://github.com/callemall/material-ui/blob/master/src/popover/popover.jsx#L203 to position: 'absolute' . Popover renders to layer, which has
this._layer.style.position = 'fixed'; this._layer.style.top = 0; this._layer.style.bottom = 0; this._layer.style.left = 0; this._layer.style.right = 0;
So basically where is no difference between fixed and absolute positioning for popover, except this issue. I will make a PR shortly. Another quesiton is, why it's in layer? Because of fixed positioning of the layer we have to handle page scroll and change popover top directly. On the other hand, if popover will has just position: absolute without layer - this issue and necessity to handle scroll will gone.

@maxpechenin
Copy link
Contributor

I don't mention, that this will not solve this issue, because layer itself has position: fixed. So i'll close the PR. So the only way which I see is to not using layer for popover or to set the positioning of layer to absolute, when popover use it.

@chrismcv
Copy link
Contributor

#3144 sorts this - we had the same issue - has become a little distracted...

@antoinerousseau
Copy link
Contributor

Got the same issue without the virtual keyboard

capture d ecran 2016-04-11 16 52 38

@nathanmarks
Copy link
Member

@newoga check this one out too RE Popover

@w01fgang
Copy link
Author

w01fgang commented Jun 8, 2016

Is there any progress? The documentation site on version 0.15.0 has the same issue.
React 15.1.0
IOS 9.3

@nsantini
Copy link

+1

@w01fgang I tested your solution and it works. The downside is that I would have to maintain a fork of material-ui and include jquery to make it work. Have you found a way to get the offset without jquery?

@w01fgang
Copy link
Author

@nsantini yes I have the way.
First of all I use react-boilerplate and for that fix I only need to take Autocomplete and Popover components, so I don't need to make a fork.
Here's the folder structure:

components
containers
material-ui/
     Autocomplete
     Popever
tests
...

And here's the function for finding needed position:

getOffsetTop (elem) => {

    let yPos = elem.offsetTop;
    let tempEl = elem.offsetParent;

    while ( tempEl != null )
    {
        yPos += tempEl.offsetTop;
        tempEl = tempEl.offsetParent;
    }

    return yPos;
}

@nsantini
Copy link

@w01fgang legend!

@antoinerousseau
Copy link
Contributor

could we make a PR with this code?

@Taakn
Copy link

Taakn commented Jul 13, 2016

I'm missing something but I can't get the fix to work. I tried using your Material UI branch @w01fgang but that didn't work either. Top position is still incorrect.

That's my autocomplete:

        <AutoComplete
          {...this.props}
          dataSource={this.state.dataSource.map((item, index) => {return item.label})}
          filter={(searchText: string, key: string) => true}
        />

Am I missing something ?

Thanks so much

@w01fgang
Copy link
Author

@Taakn can you show the output of device detection helper?

import Device from 'material-ui/utils/device';
console.log(Device);

@Taakn
Copy link

Taakn commented Jul 13, 2016

@w01fgang Thanks for your reply. I'm actually getting Error: Cannot find module 'material-ui/utils/device' so something is wrong with my configuration.

My package.json looks like this, is that correct ? Otherwise what would be the best way to get your solution ?

"material-ui": "git+https://github.com/w01fgang/material-ui.git#fixPopoverIOS",

Thanks again

@w01fgang
Copy link
Author

w01fgang commented Jul 14, 2016

@Taakn When you install from git repo, it installs in folder 'material-ui-build'

"material-ui-build": "github:w01fgang/material-ui#fixPopoverIOS",

Otherwise it doesn't installs.
Than you need to import it from material-ui-build

import AutoComplete from 'material-ui-build/AutoComplete'

@Taakn
Copy link

Taakn commented Jul 14, 2016

Hi @w01fgang thanks for your help.

When doing an npm install I am getting an unmet peer dependency error (I am using [email protected] and your package requires 0.16.0-alpha ?) but I'm not sure that would really matter.

However I am getting an Uncaught Error: Cannot find module 'material-ui-build/AutoComplete' error. When I attempt to just require('material-ui-build') my application doesn't start at all.

Do you know what could be the problem ? Please let me know if you can think of anything else.

Thanks again

@w01fgang
Copy link
Author

@Taakn sorry, my mistake. Maybe I need to sleep more 😄
Using github's version isn't simple. In npm package the material-ui is transpiled by babel, but in the github it is not. For example in my webpack config babel ignores all from node_modules folder and simple importing from material-ui-build doesn't work.
I think you can copy Popover and Autocomplete components and make a little change to them

@Taakn
Copy link

Taakn commented Jul 18, 2016

Thanks @w01fgang it took a little bit of work but it's working. It's interesting but my desktop Safari is sending the iPhone user agent when I'm debugging. Anyway thanks again!

@oliviertassinari
Copy link
Member

oliviertassinari commented Dec 10, 2016

I was digging into that issue last weekend.
I realized that the root of the issue is the iOS browser messing around with fixed positioned elements when an input is focused.

It seems that the proposed fix (#4638) is only working fine in that context but breaking the others.

I believe that the only way to solve that issue is to expose a property to enable / disable the fixed positioning.
Any thoughts?

@w01fgang
Copy link
Author

w01fgang commented Dec 10, 2016

The problem is in the fixed positioning in IOS.
The getBoundingClientRect() method on IOS returns values different from the others.
I have made this fix more than a year ago and I successfully use it.
Maybe doing refactor on the PR #4638 I broke some thing, but I don't think so, it should work.

Edit:
I have made an update to the PR.

@PhilippSpo
Copy link

There still are some issues on mobile. It seems to be related to the scroll position:
materialize-autocomplete-broken

@oliviertassinari
Copy link
Member

oliviertassinari commented Dec 30, 2016

There still are some issues on mobile. It seems to be related to the scroll position:

@PhilippSpo That's the expected behavior on an emulator. Please try it in an iPhone simulator / real device.

@ghost
Copy link

ghost commented May 6, 2017

@PhilippSpo 's issue reproduced for me on a real device too
Tho I fixed it with

popoverProps: {
                style: {
                    position: 'absolute'
                }
            }

@carlgunderson
Copy link

I'm also experiencing this issue on iOS 10.3.1 in Safari and Chrome. It's happening on the documentation site like @PhilippSpo 's example, but the Popover is appearing so far down that it's off the screen of my iPhone 6 so I can't even see it. Something seems to have changed since #4638 was implemented.

@devinivy
Copy link

I am also experiencing this issue– it's specifically related to scroll on mobile screens. I see it occurring in my Chrome devices view.

@devinivy
Copy link

devinivy commented Jun 14, 2017

For what it's worth, this doesn't seem to happen when you use the down key to enter the popover menu. As far as I can tell it seems to be directly related to the focusTextField piece of state. When it's set to false things seem to work. I tested this by setting it to false in handleChange(), and while I was not able to continue typing in the textfield, the popover did appear in the right place.

I wonder if it has something to do with the timing of setting the anchorEl when focus changes.

Edit: no progress, but I feel more sure it's related to the text field having focus.

@devinivy
Copy link

More info!

I am able to correct the issue on actual devices when the components live in a scrolling body. Originally they were in a scrolling div. Once they live as part of the scrolling body the bug appears in the Chrome devices tab but not in the iOS simulator, which is good enough for now!

TLDR when there's a focused text input in a scrolling container, and that container is scrolled, the autocomplete's popover appears where it would if the div were not scrolled.

@davidascher
Copy link

@devinivy are you saying you have a fix for this? if so, please share! ;-)

@devinivy
Copy link

My fix was to ensure the autocomplete field wasn't scrolled within a scrolling div. Instead I recreated the layout of my page to have a scrolling body. Originally the layout consisted of a full-height div that scrolled, which is what seems to have triggered the issue.

@alicerocheman
Copy link

@devinivy Geez, I cannot use your fix :/
Plus, on chrome device view, the dropdown seems to be fixed. No matter how scrolled down I am it appears in the same place, so I'm confused by your analysis... :(

@oliviertassinari
Copy link
Member

Closes by #7333 but #5544 remains.

@quorak
Copy link

quorak commented Jan 22, 2018

I think the core issue, is that older ios version dont support Element.getBoundingClientRect() which we are using here
https://github.com/mui-org/material-ui/blob/master/src/Popover/Popover.js#L264

see:
https://caniuse.com/#feat=getboundingclientrect
filamentgroup/fixed-fixed#4
lionheart/openradar-mirror#6233

@oliviertassinari oliviertassinari added the component: autocomplete This is the name of the generic UI component, not the React module! label Apr 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something doesn't work component: autocomplete This is the name of the generic UI component, not the React module! component: Popover The React component.
Projects
None yet
Development

Successfully merging a pull request may close this issue.