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

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

Open
ventsarevich opened this issue Jul 18, 2024 · 11 comments
Labels
component: charts This is the name of the generic UI component, not the React module! design This is about UI or UX design, please involve a designer enhancement This is not a bug, nor a new feature waiting for 👍 Waiting for upvotes

Comments

@ventsarevich
Copy link

ventsarevich commented Jul 18, 2024

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

@ventsarevich ventsarevich added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Jul 18, 2024
@michelengelen michelengelen changed the title [Charts]: Cannot scroll the page when touch inside the chart [charts] Cannot scroll the page when touch inside the chart Jul 18, 2024
@michelengelen michelengelen added enhancement This is not a bug, nor a new feature component: charts This is the name of the generic UI component, not the React module! and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Jul 18, 2024
@michelengelen
Copy link
Member

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
Copy link
Author

ventsarevich commented Jul 18, 2024

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

@JCQuintas
Copy link
Member

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
Copy link
Author

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
Copy link
Member

alexfauquette commented Jul 29, 2024

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

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

@michaelfaith84
Copy link

michaelfaith84 commented Aug 1, 2024

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

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

@alexfauquette
Copy link
Member

but it does seem to break drag/long press.

You're talking about the tooltip behavior?

@colinnuk
Copy link

colinnuk commented Sep 5, 2024

Before this change (#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
Copy link
Member

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

  • before 200ms (or other threshold) just let the mouse scroll
  • after 200ms block scroll and started the chart interaction (tooltip and zoom)

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

@alexfauquette alexfauquette changed the title [charts] Cannot scroll the page when touch inside the chart [charts] Allows to customize tradeof between page scroll and charts interaction on mobile Sep 5, 2024
@colinnuk
Copy link

colinnuk commented Sep 5, 2024

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 alexfauquette added design This is about UI or UX design, please involve a designer waiting for 👍 Waiting for upvotes labels Sep 9, 2024
@alexfauquette
Copy link
Member

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: charts This is the name of the generic UI component, not the React module! design This is about UI or UX design, please involve a designer enhancement This is not a bug, nor a new feature waiting for 👍 Waiting for upvotes
Projects
None yet
Development

No branches or pull requests

6 participants