[charts] Improve charts interaction for mobile users #13692
Conversation
JCQuintas
added
enhancement
|
Deploy preview: https://deploy-preview-13692--material-ui-x.netlify.app/ |
| const { x, y } = mousePosition; | ||
| const { x, y, pointerType, height } = mousePosition; | ||
| const xPosition = x; | ||
| const yPosition = y - (pointerType === 'touch' ? 40 - height : 0); |
There was a problem hiding this comment.
What is the height? I've seen it come from the event, but I don't know what it's referring to.
I'm not sure it's the correct place to add space. This yPosition tracks the pointer position. If a user what to have a different behavior they will have to take that condition into account.
For now, the sapce is added by sx={{ mx: 2 }} in the tooltip. Maybe we could move this behavior at the same place
There was a problem hiding this comment.
I explained height in the description of the PR 😅
Unsure if it would be a good idea to use mx for that, but we should probably allow it to be controllable somehow, the goal of this change is to allow the tooltip to be visible regardless of the pointer type, eg: a mouse cursor has a very thin point, so it is easy to see behind it, a thumb on the other hand would mostly hide the tooltip we want to see 😅
There was a problem hiding this comment.
I explained height in the description of the PR 😅
My bad
Another issue with modifying this point position is that tooltip is not aware of this modification. So when the tooltip does not have the space to display the content, it swiches.
Here for example the charts are at the top, preventing the tooltip to be at the top. So it displays at the bottom but still have some shift to the top (I added a red dot to show where the pointer were
There was a problem hiding this comment.
I've moved the logic to use popper.modifiers.offset, it seems to solve your concerns while keeping the functionality. Let me know what you think
| const onPointerDown = (event: React.PointerEvent) => { | ||
| if (event.currentTarget.hasPointerCapture(event.pointerId)) { | ||
| event.currentTarget.releasePointerCapture(event.pointerId); | ||
| } | ||
| }; |
There was a problem hiding this comment.
Could you explain what this code is doing?
I'm not familiar with those pointer specific events
There was a problem hiding this comment.
The if is mostly optional I believe, is just there for an early return if the pointer has been released. But in general,
if :element is capturing all the pointer events for pointer with :id
then release pointer capture for :id so it can be captured by another element :element'
In effect what it does is allow a single touch to be captured by any element it passes over rather than just the first one. 😅
Co-authored-by: Alexandre Fauquette <45398769+alexfauquette@users.noreply.github.com> Signed-off-by: Jose C Quintas Jr <juniorquintas@gmail.com>
Signed-off-by: Jose C Quintas Jr <juniorquintas@gmail.com> Co-authored-by: Alexandre Fauquette <45398769+alexfauquette@users.noreply.github.com>
Signed-off-by: Jose C Quintas Jr <juniorquintas@gmail.com> Co-authored-by: Alexandre Fauquette <45398769+alexfauquette@users.noreply.github.com>
touchActionson mobile inside the SVGpointerevents instead of high-leveltouch/mousefor better pointer capture control.topwhen we detect user is not using a mouseconst yPosition = y - (pointerType === 'touch' ? 40 - height : 0);heightis the height of the pointer itself, eg: a mouse is usually1px, a thumb is around2-5px, a touch indicator on mobile emulation is a big circle,~30px, this formula allows us to display somewhat decently in most common situations.Recoding on actual mobile
Screen_Recording_20240701_175555_Chrome.mov
fixes #13109