react-spectrum
react-spectrum copied to clipboard
Fix ios is over target
Closes https://github.com/adobe/react-spectrum/issues/1807
It seems like we can't reuse state.target in isOverTarget in different handlers since target.getBoundingClientRect() can return different values. So I save the original target.getBoundingClientRect() to state.targetRect instead.
Here is how I understand what happens:
onPointerDown- save
e.currentTargettostate.target - add
onPointerUpas global listener
- save
- pressProps.onPointerUp
- "Safari on iOS sometimes fires pointerup events, even when the touch isn't over the target, so double check.
No sure that changes we need to use
state.targetRecthere
- "Safari on iOS sometimes fires pointerup events, even when the touch isn't over the target, so double check.
No sure that changes we need to use
onPointerUp- checking
isOverTargetthe main problem is here: the coordinates of thestate.targetmight be changed. That is why we are I usingstate.targetRecthere - call
triggerPressEndwith 3rd arg (wasPressed) depends onisOverTargetresult triggerPressEnd- calls
onPressifwasPressedistrue
- calls
- checking
✅ Pull Request Checklist:
- [x] Included link to corresponding React Spectrum GitHub Issue.
- [ ] Added/updated unit tests and storybook for this change (for new code or code which already has tests).
- [x] Filled out test instructions.
- [ ] Updated documentation (if it already exists for this component).
- [ ] Looked at the Accessibility Practices for this feature - Aria Practices
📝 Test Instructions:
I was testing this using storybook. Add the following component to e.g. Button.stories.tsx.
function ButtonAlongsideWithInputAndAdditionalContent(props: any = {}) {
return <Flex direction="column">
<Flex height="size-2000">
<Text>Content</Text>
</Flex>
<TextField name="field" label="Field" placeholder="placeholder" />
<Button onPress={() => alert('pressed')}>
Button
</Button>
</Flex>
}
Also, I've left console.log if the first commit so it might be used for debugging.
🧢 Your Project:
n/a
I'm not sure that this will be correct in all cases. If a button moved between mouse/touch down and up I don't think I would expect the press event to fire, because the user's pointer is not over the target anymore.
I think perhaps this is more caused by the fact that we trigger focus to the button on touch down rather than touch up. Moving focus is what causes the keyboard to hide and the page to scroll. You can see this in your demo - touching the button without aria does not hide the keyboard until touch up, whereas the aria button it hides on touch down. Perhaps we can improve our handling there, but it will require some testing to ensure we don't break anything.
@devongovett are you saying we need to change the handling or you are going to handle that on you side? We have the bug related to this behaviour and just trying to understand how could we move forward with that issue
Either way! If you'd like to give it a try, that would be great. Otherwise, we'll look at it at some point, but I can't guarantee when.
In the meantime you may be able to use the preventFocusOnPress option to work around this. You'll need to manually focus the button in the onPress event so it receives focus on press up rather than press down.
@devongovett thanks for workaround. We decided to go this way for now
Closing this for now since there is a work around. Thanks for the input!