Improve multitouch and touch indicator behaviour on lost focus #551
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
| const [isMultiTouching, setIsMultiTouching] = useState(false); | ||
| const [isPanning, setIsPanning] = useState(false); | ||
| const [isTouchAreaActive, setIsTouchAreaActive] = useState(false); |
There was a problem hiding this comment.
do you really need this one to be expressed as state? Given you only read it in mouse event handler. Maybe you could just check document.hasFocus there?
There was a problem hiding this comment.
Right, it was unnecessary to use state there, so I've used listeners and document.hasFocus instead.
| const onFocus = () => setFocus(true); | ||
| const onBlur = () => setFocus(false); | ||
|
|
||
| window.addEventListener("focus", onFocus); |
There was a problem hiding this comment.
you may want to pass true for useCapture argument here, as if there are some elements beneath in the tree that set that listener, you'll never get it at the document level: https://developer.mozilla.org/en-US/docs/Web/API/EventTarget/addEventListener#usecapture
| useEffect(() => { | ||
| if (!focus) { | ||
| setIsPanning(false); | ||
| setIsMultiTouching(false); | ||
| setIsPressing(false); | ||
| } | ||
| setIsTouchAreaActive(focus); | ||
| }, [focus]); |
There was a problem hiding this comment.
Feels a bit weird to express this the way you did. It should be enough to have a single effect here instead of state-like variable that uses useState and useEffect under the hood just to use its output in useEffect.
Would be more readable if you just had listeners setup here, or you can modify the helper method to be
useDocuemntFocusEffect((isDocumentFocused) => {
if (!isDocumentFocused) {
// ....
}
})```
This way it wouldn't require additional state variable in this component that you never access except in the deps array for this effect here
| const [isMultiTouching, setIsMultiTouching] = useState(false); | ||
| const [isPanning, setIsPanning] = useState(false); | ||
| const [isTouchAreaActive, setIsTouchAreaActive] = useState(false); |
There was a problem hiding this comment.
isTouchAreaActive is not used, delete it?
This PR brings the following changes:
touch-areaand resuming upon re-entrytouch indicatorvisible only whentouch-areais in focus, preventing unnecessary indicator display when users switch focus to a different windowtouch markertotouch indicatorScreen.Recording.2024-09-17.at.15.53.16.mov