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

[Umbrella] React Events #15257

Open
necolas opened this Issue Mar 29, 2019 · 6 comments

Comments

Projects
None yet
2 participants
@necolas
Copy link
Contributor

commented Mar 29, 2019

This issue tracks tasks related to the React DOM implementation of the experimental React Events API (react-events). The idea is to extend React's event system to include high-level events that allow for consistent cross-device and cross-platform behavior.

Note: For now this is completely experimental and won't affect the open source builds of React.

Core

  • Optimize bundle output.
  • Document specific UX patterns that this enables and fixes. (inc. from my workplace post on styles, Dan & Brian dev tools, apps, etc.)
  • Every user-facing event should have the same standard properties, e.g., timeStamp, type, currentTarget, target. Should we provide a standard way to derive data that might be common across events and require different logic for different positional information, e.g., point, delta, offset, etc?
  • How to test discrete vs continuous events?
  • Investigate native pointer capture for retargeting events - https://www.w3.org/TR/pointerevents/#pointer-capture. This could be particularly useful for drag/swipe UX.
  • Document core API for building responders
  • Stop propagation by default but scope to event module types.
  • Investigate ways to listen to root events without first needing to wait for a target event to be dispatched to the event component.
  • Remove listener from event object passed to callbacks like onPress
  • Ensure discrete events do not unnecessarily force flush previous non-discrete events.
  • Ensure event responder get unmounted correctly and pending events (e.g. long press) properly get removed when doing so. Maybe an unmount lifecycle method needs adding to event responders?
  • Create a new synthetic event and dispatch mechanism that relies on the event being generated in the event responder module, rather than from React. This allows for the event to be better typed and have a much simpler implementation for how React fires the event when dispatched + it means we can avoid pulling in the current SyntheticEvent system and all its dependencies.

Ownership

  • Decide on prop name equivalents to RN onStartShouldSetResponder and onMoveShouldSetResponder.

Drag module

  • Determine event object data (same as Pan)
  • Cancelling drag.
  • FYI: touchAction:'none' is needed on currentTarget to prevent browser cancelling after pointermove.
  • FYI: Firefox might have problems when combining mousemove with display:flex.
  • Add README
  • Cross browser and device/modality testing.
  • Write unit tests.

Focus module

  • Determine the potential use cases for DOM focusin and focusout events.
  • Cross browser and device/modality testing.
  • add focusVisible functionality
  • Add README
  • Write unit tests
    • disabled
    • onBlur
    • onFocus
    • onFocusChange
  • Determine event object data (might require a global "modality" tracker to help attach this info to focus/blur events)
{
  pointerType: 'mouse' | 'pen' | 'touch' | 'keyboard'
}

Hover module

  • Cross browser and device/modality testing.
  • Determine event object data.
{
  pointerType: 'mouse' | 'pen',
  point: { x: number, y: number }
}
  • Write unit tests (#15283)
    • disabled
    • onHoverMove
    • onHoverChange
    • onHoverStart
    • onHoverEnd
    • delayHoverStart
    • delayHoverEnd
    • ensure doesn't respond to emulated mouse events
  • Add README
  • Add delayHoverStart and delayHoverEnd props.
  • Rename events to onHoverStart, onHoverEnd.

Pan module

  • Write unit tests.
  • Cross browser and device/modality testing.
  • Cancelling pan.
  • Add README
  • FYI: touchAction:'none' is needed on currentTarget to prevent browser cancelling after pointermove.
  • Determine event object data.
{
  pointerType: 'mouse' | 'pen' | 'touch',
  initial: { x: number, y: number },
  point: { x: number, y: number },
  velocity: { x: number, y: number },
  delta:  { x: number, y: number }
}

Press module

  • BUG: press start -> move out of target -> release -> tap press. The second press doesn't cause onPressStart/Change to be called, even thought those events are dispatched to the core responder system. So it seems to be a bug in the core?
  • Do we need to dispatch events for responder grant/release?
  • Cross browser and device/modality testing.
  • Behaviour for selecting text within pressable. End press on selection event? Add a new props to configure the behaviour, like onMoveShouldEndPress?
  • FYI: touchAction:'none' is needed on currentTarget to prevent browser cancelling after pointermove.
  • Prevent contextMenu appearing during a long press on touch screens.
  • Account for UX involving interactions on links with modifier keys held.
  • Add pressRetentionOffset to control when press is released after moving the pointer away from the press target.
  • Add README
  • Always prevent default on clicks. Maybe have an override to turn off behaviour?
  • Rename events to onPressStart, onPressEnd (#15263).
  • Change longPressCancelsPress to onLongPressShouldCancelPress (#15263).
  • Change default delayLongPress to 500ms (see note in #15290)
  • Add keyboard support for all events.
  • Prevent scroll-down page when pressing Spacebar on keyboard to interact
  • Add delayPressStart and delayPressEnd props.
  • Write unit tests (#15290)
    • disabled
    • onLongPress
    • onLongPressChange
    • onLongPressShouldCancelPress
    • onPress
    • onPressChange
    • onPressStart
    • onPressEnd
    • delayLongPress
    • delayPressStart
    • delayPressEnd
    • pressRententionOffset / pressRect
    • emulated mouse events
    • fix keyboard press events when metaKey is involved (this currently works the same way as native, so will leave it for now)
    • any special case anchor tag-related tests
    • hitslop interactions
  • Determine event object data.
{
  pointerType: 'mouse' | 'pen' | 'touch' | 'keyboard',
  initial: { x: number, y: number },
  point: { x: number, y: number },
  delta:  { x: number, y: number }
}

Scroll module

  • disabled
  • onScroll
  • onScrollDragStart
  • onScrollDragEnd
  • onMomentumScrollStart
  • onMomentumScrollEnd
  • scroll directions
  • Determine event object data

Swipe module

  • Combine onSwipe{Left,Right,Up,Down} into onSwipe w/ event data.
  • Cancelling swipe.
  • Write comprehensive unit tests.
  • Cross browser and device/modality testing.
  • Add README
  • Determine event object data (same as Pan)
  • FYI: touchAction:'none' is needed on currentTarget to prevent browser cancelling after pointermove.

Touch HitSlop

Consider whether we need this at all. Some browsers have a native hitslop and we could work with vendors on any potential improvements to the native system

  • Figure out how to position parent of hitslop.
  • Measurement without reflows (ResizeObserver?)
  • Add README
  • Figure out solution for SSR w/ non-touch interactions if no client-side JS
  • Add touchHitSlop (#15261).
  • Add touchHitSlop SSR support

Dev Tools (#15267)

  • Add displayName fields to event components and event targets.
  • Possibly add a description or displayName field to event responders? For example the Press responder module for ReactDOM could have the name ReactDOMPressResponder.
  • Expose some DEV only event triggering exports from event responder modules. i.e. HoverEventResponder.DevToolEvents = [{ name: 'hover', trigger: [{ name: 'pointerup', passive: false }]];
  • Add basic support for rendering event responders and event targets in the tree. @bvaughn

Ancillary work

  • Investigate removing object assign polyfill from individual event modules
  • Implement high-level components like Pressable (inc delays).
  • Nested Pressables should not bubble events.
  • Add internal interactive documentation / fiddle
  • Investigate press event patterns on input, textarea, select, etc.
  • Investigate accounting for element resizes when determining hit bounds, e.g., using resize observer for notifications

@trueadm trueadm added the React Flare label Mar 30, 2019

necolas added a commit to necolas/react that referenced this issue Apr 1, 2019

Add Press responder event tests
Behavior being tested takes cues from React Native's Pressability.
A couple of these tests fail and require the Press implementation to be patched.

Ref facebook#15257

necolas added a commit to necolas/react that referenced this issue Apr 1, 2019

necolas added a commit to necolas/react that referenced this issue Apr 1, 2019

Add Press responder event tests
Behavior being tested takes cues from React Native's Pressability.
A couple of these tests fail and require the Press implementation to be patched.

Ref facebook#15257

necolas added a commit to necolas/react that referenced this issue Apr 1, 2019

necolas added a commit to necolas/react that referenced this issue Apr 1, 2019

Add Press responder event tests
Behavior being tested takes cues from React Native's Pressability.
A couple of these tests fail and require the Press implementation to be patched.

Ref facebook#15257

necolas added a commit to necolas/react that referenced this issue Apr 1, 2019

necolas added a commit to necolas/react that referenced this issue Apr 2, 2019

Add Press responder event tests
Behavior being tested takes cues from React Native's Pressability.
A couple of these tests fail and require the Press implementation to be patched.

Ref facebook#15257

necolas added a commit to necolas/react that referenced this issue Apr 2, 2019

@necolas

This comment has been minimized.

Copy link
Contributor Author

commented Apr 4, 2019

Added some proposed event object data. Suggest forwarding (or creating) the pointerType on all events to allow userland customization based on modality that triggered an event; added non-standard keyboard type to Focus and Press events.

@necolas

This comment has been minimized.

Copy link
Contributor Author

commented Apr 16, 2019

Tracking some regressions I'm uncovering that seem to have been caused by the changes in 9ebe176.

  1. onPressMove is not called when moving within the responder region. Looks like no pointermove events are dispatched to the Press module in the bubble phase, which is also breaking the pressRetention functionality.
  2. No pointerleave and pointerenter events are dispatched to the Hover module in the bubble phase (discovered while investigating the Hover runtime error).
@trueadm

This comment has been minimized.

Copy link
Contributor

commented Apr 16, 2019

In relation to those two regressions: could you add tests for them that fail, but skip them with a comment explaining they need to be fixed, linking to this issue? You don’t need to put them in Press-test or Hover-test, you can make specific isolated tests with them using a custom responder to demonstrate this breaking because of the linked PR (like how we test the onUnmount works). It seems like a bug in the capture/bubbling rather than a specific event responder module.

@necolas

This comment has been minimized.

Copy link
Contributor Author

commented Apr 17, 2019

I got to the bottom of issue (1) and it turns out it's happening because different event modules can influence the propagation of the same synthetic event. In this case, Hover was preventing the propagation of pointermove to the root but Press was listening to pointermove on the root.

@trueadm

This comment has been minimized.

Copy link
Contributor

commented Apr 17, 2019

@necolas We could make root events not get affected by target propagation stopping. We can move the target and the root event handling into separate functions, so that this logic handling root events continues to run. Right now, when a target event handle stopsPropagation, the early return skips over too events too. So we'd move this block of code out:

https://github.com/facebook/react/blob/master/packages/react-dom/src/events/DOMEventResponderSystem.js#L581-L604

@necolas

This comment has been minimized.

Copy link
Contributor Author

commented Apr 17, 2019

Yeah I played around with modifying that section of code to do just that. But I think you could still end up in situations where 2 different types of event components are listening to the same event defined as a targetEventType and the inner component is stopping propagation to the outer, e.g., <Press><Hover>.... That's why I mentioned the idea of limiting control over propagation to within event components of the same type, but haven't thought through the implications enough.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.