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 Flare #15257

Closed
necolas opened this issue Mar 29, 2019 · 21 comments
Closed

[Umbrella] React Flare #15257

necolas opened this issue Mar 29, 2019 · 21 comments
Assignees

Comments

@necolas
Copy link
Contributor

necolas commented Mar 29, 2019

This issue tracks tasks related to the React DOM implementation of the experimental React Events API (react-events) aka React Flare (our internal code-name for the project). 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

  • 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 a timeStamp. 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?
  • Should we polyfill pointer events or not dispatch emulated touch/mouse events in the core, to avoid multiple event components needing to implement the same logic to ignore emulated 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.
  • Limit all event components to have a single host node as child? If not enforced for all components, we might need this for Hover/Focus/Press at least.
  • FYI Always use event capturing. Remove media events. #12919
  • Optimize bundle output.
  • Replace magic object export with React.unstable_createEventComponent(type: Symbol | number, responder: ReactEventResponder, displayName: ?string)
  • 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.

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
    • onFocusVisibleChange
  • 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 }
}

Press module

  • 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?
  • Allow contextMenu to display when element is a link and ctrl is held down during press
  • Cancel long press if active press moves (exceeding threshold, IIRC RN uses 10px delta)
  • Reactivate when moving out-of-bounds press back into the responder region.
  • 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. (Event API: Fix bug where Press root events were not being cleared #15507)
  • 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 (Rename press props in experimental event API #15263).
  • Change longPressCancelsPress to onLongPressShouldCancelPress (Rename press props in experimental event API #15263).
  • Change default delayLongPress to 500ms (see note in Add tests for Press responder event module #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 (Add tests for Press responder event module #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 }
}

FocusScope module

Consider adding onFocusIn and onFocusOut (names tbd) to support userland customisation of focus repair. We could return the native elements.

A use case to consider: being able to programmatically move focus to an element without
allowing keyboards to focus the element (e.g., the document body, the root of a modal). In this
case the element (other than a few special cases) must have tabIndex={-1}.

InputScope module

  • onChange fires when an input element has been changed. This applies to <input>, <textarea>, <select> and elements with contenteditable or when designMode is enabled. onChange provides a callback with the event for the element that changed and a key of the elment that changed (if a key was supplied).
  • onValueChange is similar to onChange, but only provides the value changed. This applies to <input>, <textarea>, <select> and elements with contenteditable or when designMode is enabled. onValueChange provides a value and key of the element that changed (if a key was supplied).
  • onSubmit for when any <form> elements trigger form submit.
  • onKeyPress for when any keyboard keys are pressed.
  • preventKeys accepts an array of key strings that will get prevented from entering input.
  • onSelectionChange for when any any text selection occurs in any child elements.
  • onBeforeChange fires before a change is about to occur. This applies to <input>, <textarea>, <select> and elements with contenteditable or when designMode is enabled. onBeforeChange provides a callback with the event for the element that changed and a key of the elment that changed (if a key was supplied).

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.

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 }
}

Scroll module

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

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
  • Add internal interactive documentation / fiddle
  • Investigate press event patterns on input, textarea, select, etc.
  • Implement high-level components like Pressable (inc delays).
  • Nested Pressables should not bubble events.
  • Investigate accounting for element resizes when determining hit bounds, e.g., using resize observer for notifications
necolas added a commit to necolas/react that referenced this issue Apr 1, 2019
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
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
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
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
Copy link
Contributor Author

necolas 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.

@TrySound
Copy link
Contributor

TrySound commented May 9, 2019

Hi guys. Great work! Will React.unstable_createEventComponent support observers api like ResizeObserver and IntersectionObserver?

@trueadm trueadm changed the title [Umbrella] React Events [Umbrella] React Flare Jun 4, 2019
@trueadm
Copy link
Contributor

trueadm commented Jun 7, 2019

@TrySound Event responder modules are free to use whatever DOM APIs they want, so they could definitely use ResizeObserver and IntersectionObserver. We may even need to use them in some of the core responders soon.

@Andarist
Copy link
Contributor

Question regarding potential scroll module - have you considered adding smth like .userInduced property? It's highly useful to know if the scroll was triggered programatically or by user and it's also highly hard to determine this in user space.

@necolas
Copy link
Contributor Author

necolas commented Jun 14, 2019

What's an example of scroll being programmatically triggered without your knowledge?

@TryingToImprove
Copy link
Contributor

@necolas I feel like every change to scrollTop in code is not user introduced.

@necolas
Copy link
Contributor Author

necolas commented Jun 14, 2019

But that's easy to detect because you're using a programmatic API

@gaearon
Copy link
Collaborator

gaearon commented Jun 14, 2019

It does get a bit difficult when the scrolling trigger is far from the listener, and making them aware of each other introduces fragile coupling. We've had a few such cases in DevTools. Especially if the focus signal is reactive to some state change.

@necolas
Copy link
Contributor Author

necolas commented Jun 14, 2019

Here's what I ask: if you have a request please take the time to provide plenty of context. What are the use cases? How are things difficult? How do you imagine a solution might work? etc.

@Andarist
Copy link
Contributor

What are the use cases?

In example showing a "sticker" with a date at the top of the message feed. When a scroll is induced programmatically (or by the browser) then it's distracting to show such thing, but when a user scrolls on their own it makes sense from the UX perspective.

How are things difficult?

Well, it's not a clear way of determining that in onScroll, so some kind of other tracking flags have to be introduced to solve this. It's in general tricky to handle all possible reasons of why scroll has been triggered:

  • it can be caused by wheel (but also wheel doesnt always mean that scroll will happen - u can have other interactions dependant on the wheel), but also you can wheel over inner scrollable element which won't trigger the scroll of the component in question
  • touchmove - but you probably have to calculate this based on some deltas etc
  • keyboard - space, page up, page down, end, home, arrows

And those are just things that user can do. Other than that UI might get mutated (even by React, so we can't really track it in userland) and thus trigger scroll event, and as @gaearon mentioned setting scrollTop also triggers scroll event.

And even if u manage to track all reasons of user-induced scrolling it's unobvious when to clear you your flag(s) because you have to consider inertia scrolling etc

How do you imagine a solution might work?

The solution would be to figure out best heuristics of detecting this. Can't give much detail right now, because this truly is an overcomplicated problem for such a "simple" thing. Web ❤️

@necolas
Copy link
Contributor Author

necolas commented Jun 14, 2019

I don't quite understand the use case you're describing.

@Andarist
Copy link
Contributor

Consider another one - a floating "scroll to bottom" button which covers part of the chat feed. We'd like to show such when a user is scrolling on their own (with a delay etc) and hide it after they stop scrolling so we don't cover part of the chat feed constantly. And we i.e. have to work around for situations like mobile keyboard triggering onScroll (cause of the resize etc).

@craigkovatch
Copy link

Given the grand retooling happening to the event system here, I would like to re-raise the following request under this umbrella, from #3751 (comment):

TL;DR: listen to focusin/focusout rather than focus/blur native events.

But that is not the issue, IE 9-11 does support relatedTarget. The problem is that React binds the wrong event. React listens to events that don't have relatedTarget instead of the proper events that do have relatedTarget. This request never asked for React to attempt to polyfill something that wasn't available, it only asked for React to listen to the correct event (which it already had code targeting IE8 to handle that).
This discrepancy has only gotten worse over time. To my knowledge Firefox fixed its issue and implemented relatedTarget, removing the issue of one browser not supporting it resulting in all browsers supporting it. IE11 however has stuck around, yet relatedTarget doesn't work in React despite IE11 like all other browsers with notable userbases supporting relatedTarget.

@AlfredoGJ
Copy link
Contributor

Hey guys, looks pretty cool this experimental event system. I wonder if there is still work to do, I would like to contribute to this.

I see there are some pending tasks in the checklist but no open issues about them

@necolas
Copy link
Contributor Author

necolas commented Dec 13, 2019

We're not longer working on this particular approach and will be exploring other ways to work with the DOM event system in the future. We've concluded that the "Flare" event system is too high-level an abstraction and we'd like to explore something that is a bit more familiar to developers familiar with the DOM (e.g., addEventListener) and React's existing tools (e.g., hooks). Our goal is still to make it possible for library authors to work with passive events, capture/bubble phase, custom events, and events occurring on the document from within a React function component...while reducing the amount of event-related code needed in ReactDOM. Ultimately building both intermediate abstractions like the Responder Event system and high-level abstractions like useTap (which we prototyped in Flare) should be possible in user-space.

@Andarist
Copy link
Contributor

That's a bummer - this API looked awesome and I had high hopes for it. I guess you have good reasons for not pursuing this, I'm eager to learn with what alternative you will come up with.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants