Open TimVDAQ opened 9 months ago
TimVDAQ
commented
9 months ago 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)
oliviertassinari
commented
9 months ago For the sake of linking to the history of this logic: https://github.com/mui/material-ui-pickers/pull/1653#issuecomment-614823334 & https://github.com/mui/material-ui/issues/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 https://github.com/mui/material-ui/issues/15000#issuecomment-497125867, 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
commented
9 months ago Something I found randomly, looking for something else:
GitHub is using:
'@media (hover: hover) and (pointer: fine)': {
LukasTy
commented
9 months ago Something I found randomly, looking for something else:
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. 👌
oliviertassinari
commented
8 months ago A deeper dive https://hoverpointer-media-query.glitch.me, using BrowserStack:
pointer: fine is wronghover: hover is wrong https://www.ctrl.blog/entry/css-media-hover-samsung.htmlLukasTy
commented
8 months ago 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
commented
7 months ago @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:
LukasTy
commented
7 months ago @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
commented
7 months ago 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
commented
7 months ago
- We could almost have a new docs page, like "Limitations" https://mui.com/material-ui/react-autocomplete/#limitations.
That is a good point. Especially if we would be adding this information to the doc, then refactoring this section could make sense.
- The callouts format in https://mui.com/x/react-date-pickers/base-concepts/#testing-caveats feels strange, it's already inside a caveat section. It breaks a bit the reading flow.
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. 🤔
LukasTy
commented
7 months ago 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.
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).
Steps to reproduce 🕹
Link to live example:
Steps:
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 https://github.com/ionic-team/ionic-framework/issues/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 🌎
``` 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 ```npx @mui/envinfoRunning the latest update to Google Chrome on the Google Pixel 7