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

[data grid] Prevent diagonal scrolling on iOS #11230

Open
oliviertassinari opened this issue Nov 28, 2023 · 13 comments
Open

[data grid] Prevent diagonal scrolling on iOS #11230

oliviertassinari opened this issue Nov 28, 2023 · 13 comments
Labels
component: data grid 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 mobile Targets mobile platform

Comments

@oliviertassinari
Copy link
Member

oliviertassinari commented Nov 28, 2023

This is a continuation of the discussion that started at #10059 (comment) about the scrolling experience on mobile. The second comment about this: #10059 (comment).

I think that we all agree that the best UX on mobile is one where the scrolling direction is relatively well "fixed".

The test environment: https://next.mui.com/x/react-data-grid/#pro-plan.

You can find another example with https://docs.grid.glideapps.com/api/dataeditor/input-interaction#preventdiagonalscrolling. Implemented here. Related issue: glideapps/glide-data-grid#27.

On Android, the native scroll feels "right" ✅, it has a logic that locks the scroll in one direction once you are into it, but still lets you easily change the scroll direction with a large enough finger movement and without having to touch up and down. I don't think we should change it.

On iOS, the native scroll experience feels "broken" ❌, it's too hard to scroll vertically or horizontally without "drifting". We should fix that.

mui/material-ui#20990 is a bit related but a different problem, though it raises how touch-action could be part of the solution.

Benchmarks

  • https://grid.glideapps.com/: macOS seems good. Android is a bit weird, it's too hard to change scroll direction after some time. iOS, same feeling as on Android. Overall, it's close to what I think we should expect.
  • AG Grid ❌: feels "broken", it locks me in a specific scroll direction. It's not as broken as iOS by default (so a step forward) but it's definitely not as great as Android.
@oliviertassinari oliviertassinari added component: data grid 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 labels Nov 28, 2023
@oliviertassinari oliviertassinari changed the title [data grid] Prevent diagonal scrolling on mobile? [data grid] Prevent diagonal scrolling on iOS Nov 28, 2023
@oliviertassinari oliviertassinari added the enhancement This is not a bug, nor a new feature label Nov 29, 2023
@romgrk
Copy link
Contributor

romgrk commented Dec 7, 2023

I think this could be extended to the desktop as well.

When I'm scrolling say vertically, it often happens that the viewport will deviate slightly horizontally as I don't scroll perfectly straight. It would feel better if it didn't happen, not only for the UX but also because the horizontal scroll can trigger a column re-render and that can be very expensive if the user is scrolling vertically at the same time.

The implementation would need to be really good, because otherwise it can make the grid very annoying to use. For example, in ag-grid the scroll-lock is kept active as long as the scroll hasn't completely stopped for maybe 0.75-1.0 seconds but the timer is extended even by user inputs that don't moved the viewport, so if I scroll vertically then directly switch to horizontal, I can keep inputing scroll gestures forever and it will not move the viewport until I stop for 1 second. That's obviously a very bad implementation.

I think we should have the scroll-lock implement just a tiny bit of resistance. On possible implementation could be that the scroll-lock switches only when one of the x/y components of the wheel or touchmove event becomes bigger than the other. E.g. if dx = -100 and dy = +0.5, we can safely ignore the dy.

@oliviertassinari
Copy link
Member Author

oliviertassinari commented Dec 7, 2023

to the desktop as well. When I'm scrolling say vertically

@romgrk Do you mean on Windows? I can't reproduce it on macOS (desktop).

should have the scroll-lock implement just a tiny bit of resistance.

Agree. For me, macOS is great, it can be the target for mouse interactions. Android is great, it can be the target for touch interactions.

On macOS (desktop), it feels like the implementation relies on:

  • if dx < dy then the scroll lock direction is vertical
  • if dx > dy then the scroll lock direction is horizontal

On Android, it feels like the implementation relies on:

  • if dx * 2 < dy then the scroll lock direction is vertical
  • if dx > dy * 2 then the scroll lock direction is horizontal
  • otherwise, you can scroll in both directions

In these two, dx, dy feels computed over the span of the last 10 events.

@romgrk
Copy link
Contributor

romgrk commented Dec 7, 2023

Yeah I'm on chrome linux, but usually it behaves the same as chrome windows.

@lauri865
Copy link
Contributor

lauri865 commented Dec 9, 2023

That would be awesome. It's impossible to scroll in straight line on iPhone.

Furthermore, you may want to disable overscroll-behavior. Every time I visit mui-x on my iPhone, I think the virtualization is laggy/broken because of overscroll-behavior. Try scrolling up/down and pull a bit to the right (so that it overscrolls on the left-side), and you'll notice how next block of rows will display only after a visible delay, and at once.

No issues on the desktop though. The experience is great on MacOS, and would be great not to have any artifical delays to the native behaviour there.

@lucare
Copy link

lucare commented May 4, 2024

Did this feature get implemented? I like to switch to MUI X for a project, but both the diagonal scroll and over scroll bounce are what are giving me pause at the moment. I would really like to use MUI X Premium (since I also need grouping) since otherwise MUI X is much faster and than what I am currently using.

@oliviertassinari
Copy link
Member Author

oliviertassinari commented May 4, 2024

@lucare Out of curiosity, what are you using now that is too slow?

As for this issue, I wouldn't expect this to be super hard to fix. For example, I had to solve problems like this in the past https://github.com/oliviertassinari/react-swipeable-views/blob/be57b7d9ba34c349a725b5ce9c0b265e0a9a9c18/packages/react-swipeable-views/src/SwipeableViews.js#L420-L440 or mui/material-ui#20990 (comment), I don't recall it was a big effort.

Maybe this could be a cool item as part of the onboarding of @KenanYusuf in the team 😄

@lauri865
Copy link
Contributor

lauri865 commented Jan 23, 2025

As for this issue, I wouldn't expect this to be super hard to fix. For example, I had to solve problems like this in the past

Famous last words 🙈. This seems to be an incredibly difficult problem to solve.

Did a few explorations:

  1. Manipulating scroll position while scrolling stops scrolling in all directions, breaking momentum
  2. Thought about touch-actions pan-y and pan-x, but they don't seem to work well either
  3. Having nested containers (ag-grid uses this) – one with overflow-y, the other with overflow-x – it doesn't work with position: sticky + it adds an annoyingly long lock as @romgrk pointed out (that's browser specific, not sure it can be overcome)
  4. Only option left seems to be: custom scroll logic (only for touch devices, doing this for mousewheel is a criminal offense) with custom momentum implementation. The problem is, animating scrollLeft/scrollTop is a performance nightmare.
  5. Last resort: using transform on a child of the scroller. But then you need to start syncing transforms for many elements (top container, render zone, bottom container, can't wrap them, since that would break position sticky) And if that's not enough, you will now have two scroll positions that you need to sync / switch between – what if an iPad user uses both touch and a mouse? Also, what about all the current implementations for scrollToIndexes, scrollIntoView, etc.

Haven't found any app that has solved it while keeping native scrolling for desktop and without using canvas.

@oliviertassinari
Copy link
Member Author

criminal offense

@lauri865 😂. I have seen some grid manipulating the scroll so much it was horrible. Thanks for the report on the exploration. Path 3 seemed to have potential, maybe it could solve #13551 too. Giving up on a sticky positioning, hum, yeah, hard UX tradeoff.

For 4. and 5. this feels like a dead end.

@lauri865
Copy link
Contributor

lauri865 commented Jan 23, 2025

criminal offense

@lauri865 😂. I have seen some grid manipulating the scroll so much it was horrible. Thanks for the report on the exploration. Path 3 seemed to have potential, maybe it could solve #13551 too. Giving up on a sticky positioning, hum, yeah, hard UX tradeoff.

For 4. and 5. this feels like a dead end.

I don't think path 3 has any merit. The browser can lock the scroll for a full second before you can switch directions, plus giving up position sticky for this is a bad tradeoff imho. Then you get out of sync body and headers like Ag-grid has, and like Mui Datagrid used to have.

My hack on path 5 is testable here (if you have a touch device at hand; has many rough edges, but the UX is still better than 3rd option would give us imho) (ref #16313):
https://deploy-preview-16313--material-ui-x.netlify.app/x/react-data-grid/#pro-version

@romgrk
Copy link
Contributor

romgrk commented Jan 23, 2025

Setting overflow-[n]: hidden seems to lock scrolling to a single axis while preserving inertial scrolling (at least on chrome linux), might be interesting to consider: https://jsfiddle.net/e3qt7ucd/5/. It makes the real scrollbar disappear but we don't care about that because we use virtual scrollbars.

@lauri865
Copy link
Contributor

lauri865 commented Jan 23, 2025

Setting overflow-[n]: hidden seems to lock scrolling to a single axis while preserving inertial scrolling (at least on chrome linux), might be interesting to consider: https://jsfiddle.net/e3qt7ucd/5/. It makes the real scrollbar disappear but we don't care about that because we use virtual scrollbars.

Can't see the demo on my phone, but that was one of the first things I tested (forgot to mention).

Problems:

  1. Need to keep both directions scrolling in the beginning of the scroll to detect direction to preserve inertia. Meaning before you lock, you can still scroll diagonally. Any manual overrides I tried ended up in flickering.
  2. Toggling overflow resets scroll in that direction (e.g. if you lock horizontally, but are already scrolled down, the scrollTop will reset to 0). And I think also broke inertia for me on iOS (regardless of the direction), but can't recall anymore.

I didn't see clear path to solve these, but if there is, it'd be great of course.

Just wish there was a 'beforeScroll' event that'd enable manipulating scroll position before the frame is painted.

Edit:
Works only horizontally on iOS, vertically, it just stops / jumps / janks for me:
https://output.jsbin.com/horizeguje

@romgrk
Copy link
Contributor

romgrk commented Jan 23, 2025

Works only horizontally on iOS, vertically, it just stops / jumps / janks for me:

Does fiddling with the if-check changes anything on iOS? The code I posted checks for dl !== 0 first (horizontal lock), maybe the check can be tweaked to make it work. I don't have an iOS device to test with :/

@lauri865
Copy link
Contributor

lauri865 commented Jan 23, 2025

I added a calculation for direction:
https://jsbin.com/xuxuvegulo/2/edit?html,css,js,output

Output:
https://jsbin.com/xuxuvegulo/2

It only ever works in one direction, then it gets jammed. Does it work with Android? Or does Android have scroll railing built in?

Edit: as long as there's offset in the opposite direction, the scroll is jammed and jumps back up or to the left a bit. If the offset clears, it works again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: data grid 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 mobile Targets mobile platform
Projects
Status: 🆕 Needs refinement
Development

No branches or pull requests

5 participants