Fix: Add mobile tap support for CustomTooltip #1031
Conversation
Signed-off-by: M-DEV-1 <mahadevankizhakkedathu@gmail.com>
Screencast.from.2025-04-30.18-36-57.webmI tested the above changes, and it works for Modal Footer icon, CustomToolTips, InfoToolTips 👍🏼 |
|
Thank k you, @M-DEV-1 |
There was a problem hiding this comment.
This is good change . @M-DEV-1 do you mind taking look to the reviews
| @@ -27,6 +27,21 @@ function CustomTooltip({ | |||
| componentsProps = {}, | |||
| ...props | |||
| }: CustomTooltipProps): JSX.Element { | |||
| const [isOpen, setIsOpen] = useState(false); | |||
|
|
|||
| const isTouchDevice = () => { | |||
There was a problem hiding this comment.
i think isTouchDevice will run on every render. This is not good. Do you mind move it to useMemo .
There was a problem hiding this comment.
Yes, you're right. I'll move to useMemo
| setIsOpen(true); | ||
| }; | ||
|
|
||
| const handleTouchEnd = () => { |
There was a problem hiding this comment.
yes, adding debounce will prevent bad behavior @sudhanshutech
Signed-off-by: M-DEV-1 <mahadevankizhakkedathu@gmail.com>
| const isTouchDevice = useMemo(() => { | ||
| return 'ontouchstart' in window || navigator.maxTouchPoints > 0; | ||
| }, []); | ||
|
|
||
| const handleTouchStart = (event: React.TouchEvent<HTMLDivElement>) => { | ||
| event.preventDefault(); | ||
| setIsOpen(true); | ||
| }; | ||
|
|
||
| const handleTouchEnd = useMemo( | ||
| () => | ||
| _.debounce(() => { | ||
| setIsOpen(false); | ||
| }, 1500), | ||
| [setIsOpen] | ||
| ); | ||
|
|
There was a problem hiding this comment.
@M-DEV-1 why we required debounce and ontouchstart things
There was a problem hiding this comment.
use this sort of implmentation for mobile view
<ClickAwayListener onClickAway={handleTooltipClose}>
<div>
<Tooltip
onClose={handleTooltipClose}
open={open}
disableFocusListener
disableHoverListener
disableTouchListener
title="Add"
slotProps={{
popper: {
disablePortal: true,
},
}}
>
<Button onClick={handleTooltipOpen}>Click</Button>
</Tooltip>
</div>
</ClickAwayListener>
There was a problem hiding this comment.
I added debounce and useMemo because I had implemented isTouchDevice for mobile device detection. I'll use a different approach, one with using MUI's default touch delay and the implementation you have given. Thanks
There was a problem hiding this comment.
try to do same thing that implemented in mui tooltip click varient
|
@M-DEV-1 any updates? |
Notes for Reviewers
This PR fixes #1005
Signed commits