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

[pickers] Some mobile phones render desktop pickers (i.e. Google Pixel 7) #10039

Open
TimVDAQ opened this issue Aug 15, 2023 · 11 comments
Open
Labels
component: pickers This is the name of the generic UI component, not the React module! docs Improvements or additions to the documentation enhancement This is not a bug, nor a new feature good first issue Great for first contributions. Enable to learn the contribution process. mobile Targets mobile platform

Comments

@TimVDAQ
Copy link

TimVDAQ commented Aug 15, 2023

Steps to reproduce 🕹

Link to live example:

Steps:

  1. Create a page with a DatePicker and/or TimePicker component in it
  2. Open page on Google Pixel 7 phone
  3. Compare outcomes with other devices, a desktop environment and the expected behavior.

This appears to be due to Google Pixel 7 phones returning true to the pointer: fine media query when all other phones return false.
See this PR on another package for more information ionic-team/ionic-framework#26240

Current behavior 😯

On Google Pixel 7 phones, the desktop version of the date and time pickers are displayed.

Expected behavior 🤔

On Google Pixel 7 phones, the mobile version of the date and time pickers should be displayed, in line with how other phones display the date and time pickers.

Context 🔦

I am unable to properly provide the mui date and time picker mobile interface to Google Pixel 7 users

Your environment 🌎

npx @mui/envinfo
  System:
    OS: Windows 10 10.0.22621
  Binaries:
    Node: 18.16.0 - C:\Program Files\nodejs\node.EXE
    Yarn: 1.22.19 - ~\AppData\Roaming\npm\yarn.CMD
    npm: 9.5.0 - C:\Program Files\nodejs\npm.CMD
  Browsers:
    Chrome: Not Found
    Edge: Spartan (44.22621.1992.0), Chromium (115.0.1901.203)
  npmPackages:
    @emotion/react:  11.11.1
    @emotion/styled:  11.11.0
    @mui/base:  5.0.0-beta.10
    @mui/core-downloads-tracker:  5.14.4
    @mui/material:  5.14.4
    @mui/private-theming:  5.14.4
    @mui/styled-engine:  5.13.2
    @mui/system:  5.14.4
    @mui/types:  7.2.4
    @mui/utils:  5.14.4
    @mui/x-date-pickers:  6.11.0
    @types/react:  18.2.20
    react:  18.2.0
    react-dom:  18.2.0
    typescript: ^5.1.6 => 5.1.6

Running the latest update to Google Chrome on the Google Pixel 7

@TimVDAQ TimVDAQ added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Aug 15, 2023
@TimVDAQ
Copy link
Author

TimVDAQ commented Aug 15, 2023

I was able to fix this using the desktopModeMediaQuery prop, however the default behaviour should still be changed.

A fix is to replace the default media query @media (pointer: fine) with the query @media (hover: hover) and (pointer: fine)

@zannager zannager added component: pickers This is the name of the generic UI component, not the React module! mobile Targets mobile platform labels Aug 15, 2023
@oliviertassinari
Copy link
Member

For the sake of linking to the history of this logic: mui/material-ui-pickers#1653 (comment) & mui/material-ui#15000.

A fix is to replace the default media query @media (pointer: fine) with the query @media (hover: hover) and (pointer: fine)

If this works, it would pretty be the same as mui/material-ui#15000 (comment), then I think we could apply the same logic everywhere. I have a similar issue on Material UI buttons with my Samsung phone, I see the hover style, it's annoying.

@oliviertassinari
Copy link
Member

oliviertassinari commented Aug 16, 2023

Something I found randomly, looking for something else:

https://github.com/primer/react/blob/66482a72000a0f1baf021e2b554e98942081d685/src/ActionList/Item.tsx#L111

GitHub is using:

'@media (hover: hover) and (pointer: fine)': {

@LukasTy
Copy link
Member

LukasTy commented Aug 17, 2023

Something I found randomly, looking for something else:

https://github.com/primer/react/blob/66482a72000a0f1baf021e2b554e98942081d685/src/ActionList/Item.tsx#L111

Thank you for pointing out this resource, it had confirmed the direction that it makes sense updating the rule to be more strict and reduce the false-positives that we currently encounter due to device reporting differences. 👌

@LukasTy LukasTy self-assigned this Aug 17, 2023
@LukasTy LukasTy added enhancement This is not a bug, nor a new feature and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Aug 17, 2023
@LukasTy LukasTy changed the title Mobile phone state is not correctly detected on Google Pixel 7 and 7 Pro phones (other phones may be affected) [pickers] Some mobile phones render desktop pickers (i.e. Google Pixel 7 and 7 Pro) Aug 17, 2023
@LukasTy LukasTy changed the title [pickers] Some mobile phones render desktop pickers (i.e. Google Pixel 7 and 7 Pro) [pickers] Some mobile phones render desktop pickers (i.e. Google Pixel 7) Aug 17, 2023
@oliviertassinari
Copy link
Member

oliviertassinari commented Aug 23, 2023

A deeper dive https://hoverpointer-media-query.glitch.me, using BrowserStack:

  • ❌ Google Pixel, pointer: fine is wrong
Screenshot 2023-08-24 at 01 02 09 Screenshot 2023-08-24 at 01 01 40
  • ✅ Xiaomi, correct
Screenshot 2023-08-24 at 01 01 07
  • ✅ OnePlus, correct
Screenshot 2023-08-24 at 01 02 47
  • ✅ iOS, correct
Screenshot 2023-08-24 at 01 12 03

@LukasTy LukasTy removed their assignment Aug 25, 2023
@LukasTy
Copy link
Member

LukasTy commented Aug 25, 2023

This looks to be a somewhat common problem among many devices and/or browsers.
Some have gone away and implemented solutions to avoid completely misrepresenting the capabilities.

Technically, IMHO, it's not our concern to come up with hacky solutions to fix such problems and the proposed solution in PR could backfire on us in certain scenarios that we are not aware of yet.

Maybe just adding a documentation section about these edge cases would be a good trade-off? 🤔

I'm putting the issue up for grooming.

@oliviertassinari
Copy link
Member

oliviertassinari commented Sep 24, 2023

@LukasTy Tricky.

In the case of the hover style, I think that '@media (hover: hover) and (pointer: fine)': { seems fair.

However, in the case of flipping between the mobile and desktop variants, it feels better to:

  • make sure the desktop version works on mobile
  • have developers customize the toggle if they want a different tradeoff

@LukasTy
Copy link
Member

LukasTy commented Sep 25, 2023

@oliviertassinari Below are replies to your points:

  • make sure the desktop version works on mobile

I'm pretty sure that desktop pickers do not have problems working on mobile. 🤔

  • have developers customize the toggle if they want a different tradeoff

That is already pretty easy to do with desktopModeMediaQuery prop. 👍

TL/DR: My gut tells me that we should not change anything regarding the out of the box behavior to cater to buggy implementations, because we'd eventually arrive to the age-old IE problem. 😆

WDYT about adding more information in the relevant documentation page and mentioning it in the callouts in addition to the already mentioned testing responsiveness case?

@oliviertassinari
Copy link
Member

behavior to cater to buggy implementations, because we'd eventually arrive to the age-old IE problem.

Kendo jQuery success was partially based on browser interoperability that used to be a mess. @media (hover: hover) and (pointer: fine) sounds fair, so depends.

WDYT about adding more information in the relevant documentation page and mentioning it in the callouts in addition to the already mentioned testing responsiveness case?

@LukasTy
Copy link
Member

LukasTy commented Sep 25, 2023

That is a good point. Especially if we would be adding this information to the doc, then refactoring this section could make sense.

Good point, at this point and the current structure of the docs, the callout is probably redundant, we could aim at making the information on the main page as discoverable as possible. 👍

  • I think to consider how much a new docs section helps when searching. If this GitHub issue already easily surfaces on Google, it might not be needed.

Good point, but I tried searching for mui-x date picker renders desktop variant on mobile in Incognito to no avail, this issue hasn't popped up. 🙈 IMHO, it wouldn't hurt putting this info in the doc to increase awareness. 🤔

@alexfauquette alexfauquette added the docs Improvements or additions to the documentation label Oct 3, 2023
@LukasTy
Copy link
Member

LukasTy commented Oct 3, 2023

We decided to go with improving the existing responsive callouts by relocating them to either a more visible section in https://mui.com/x/react-date-pickers/base-concepts or even a new page (Limitations) as suggested.
Screenshot 2023-10-03 at 12 07 06

We'd be extending the mentioned section/page with the information about possible issues on certain devices just like this one, preferably linking to most relevant issue(s).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: pickers This is the name of the generic UI component, not the React module! docs Improvements or additions to the documentation enhancement This is not a bug, nor a new feature good first issue Great for first contributions. Enable to learn the contribution process. mobile Targets mobile platform
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants