search

mui / mui-x

MUI X: Build complex and data-rich applications using a growing list of advanced React components, like the Data Grid, Date and Time Pickers, Charts, and more!
https://mui.com/x/
4.53k stars 1.32k forks source link

[charts] Allows to customize tradeof between page scroll and charts interaction on mobile #13885

Open ventsarevich opened 4 months ago

ventsarevich commented 4 months ago

Steps to reproduce

Check the video: https://github.com/user-attachments/assets/5317df9b-9669-43a0-ac94-ad97438730e5

There is no option to scroll horizontally or vertically over the chart on mobile devices.

Has some option to enable scroll over on mobiles?

Current behavior

Can't scroll the page when touch the chart

Expected behavior

Provide an ability to disable that behavior to have the ability to scroll over the large charts.

Context

Check the video: https://github.com/user-attachments/assets/5317df9b-9669-43a0-ac94-ad97438730e5

There is no option to scroll horizontally or vertically over the chart on mobile devices. It's inconvenient for cases when charts is large and not scollable

Your environment

System: OS: macOS 14.5 Binaries: Node: 20.5.0 - ~/.nvm/versions/node/v20.5.0/bin/node npm: 9.8.1 - ~/WebstormProjects/Barrio/client/node_modules/.bin/npm pnpm: 9.4.0 - /opt/homebrew/bin/pnpm Browsers: Chrome: 126.0.6478.127 Edge: Not Found Safari: 17.5 npmPackages: @emotion/react: ^11.11.4 => 11.11.4 @emotion/styled: ^11.11.5 => 11.11.5 @mui/base: 5.0.0-beta.40 @mui/core-downloads-tracker: 5.15.18 @mui/icons-material: ^5.15.18 => 5.15.18 @mui/material: ^5.15.18 => 5.15.18 @mui/private-theming: 5.16.4 @mui/styled-engine: 5.16.4 @mui/system: 5.16.4 @mui/types: 7.2.15 @mui/utils: 5.16.4 @mui/x-charts: ^7.10.0 => 7.10.0 @mui/x-date-pickers: ^7.5.0 => 7.5.0 @types/react: 18.0.28 => 18.0.28 react: 18.2.0 => 18.2.0 react-dom: 18.2.0 => 18.2.0 typescript: ~4.9.5 => 4.9.5

Browser: Safari (IOS)

Search keywords: mobile, scroll, chart

michelengelen commented 4 months ago

Hey @ventsarevich ... thanks for raising this issue. I will add this to the board, but could you tell us if this is restricted to only mobile devices? And if yes, does it work on another OS?

ventsarevich commented 4 months ago

This bug is relevant only to mobile devices. I reproduced this issue on IOS devices on Chrome and Safari and Android Chrome.

JCQuintas commented 4 months ago

This was caused by a change to make users more easily see tooltips on mobile. We made it so you can drag your finger around the chart and the tooltip will show up as you drag. Maybe we need to think of an alternative... 🤔

ventsarevich commented 4 months ago

Having some flags to set the tooltip view option would be great. In some cases, showing a tooltip by dragging a finger over the chart could be convenient. But there are cases like a large chart or horizontal scroll when that behavior is just annoying and inconvenient.

alexfauquette commented 3 months ago

@ventsarevich Would removing touch-action: none; on the <svg/> solves your issue?

Could be done with sx={{'&&': {touchAction: 'auto'}}}

michaelfaith84 commented 3 months ago

@alexfauquette That seems to have done it for me! Thanks!

Edit: but it does seem to break drag/long press.

alexfauquette commented 3 months ago

but it does seem to break drag/long press.

You're talking about the tooltip behavior?

colinnuk commented 2 months ago

Before this change (https://github.com/mui/mui-x/pull/13692), I'd written a wrapper component which would intercept touchstart, touchmove & touchend events and send them to MUI as mouse events.

The component would only start sending the mouse events if a time threshold had passed (200ms). This meant that a user would 'touch and hold' and could then navigate the charts with the tooltip, or if they started scrolling (panning) then the mouse events would never fire, the tooltip wouldn't show up and the user would instead scroll the page. This just about worked ok - despite the hacky approach.

The PR linked above (while no doubt better for most users of this library) breaks this functionality completely.

The charts on my mobile page span the whole width of the page, so if I want the tooltip behaviour (which I do) - then the user is unable to scroll if they happen to touch a chart first, which isn't immediately obvious to them.

I think some form of threshold would be good - so if a user is long pressing they get the nice tooltip function, and if they only tap for a very short time then they can still scroll & pan the page.

alexfauquette commented 2 months ago

@colinnuk Thanks for the nicely detailed proposal. If I get it wrigh you propose to have timer starting at the touch down such that

Sounds like a nice behavior for the tooltip. I'm a bit more concerned by the pan for pro charts since those are also a scroll behavior

I'm also not sure about how to implement it, because it would require to remove the touchAction: 'none' leading to tons of edge cases where we need to take care of preventing default scroll

colinnuk commented 2 months ago

Yeah I don't see how it would be combined well with pan & zoom functionality.

After this change I was trying to ceate a wrapper which would change the touchAction from 'auto' to 'none' after a timeout, but it doesnt seem to work.

I'm also not sure about how to implement it, because it would require to remove the touchAction: 'none' leading to tons of edge cases where we need to take care of preventing default scroll

I had to use event.preventDefault() for this to work.

In the meantime I think I am just going to implement a switch function to 'focus' on the graph with a 'close' button. I've seen this elsewhere (Garmin Connect app), where to get the detailed tooltip info you have to click on the graph which focuses the screen on that one graph and then provides a close/back button to go back to the main screen.

EDIT: Example here

image
alexfauquette commented 2 months ago

Thanks for the detailed explanation. We are going to do more exploration on how other website are managing this tradeoff on this aspect to come with a robust solution.