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

Feature: add element dependency with events #81

Merged
merged 10 commits into from Jul 30, 2019

Conversation

@divyanshu013
Copy link
Member

divyanshu013 commented Jun 28, 2019

Reference

Spec

This feature introduces a new trigger - on-event and a new behavior action dispatch-event. (Refer to the example for complete XML)

Consider the following view rendered on the screen:

<view action="append" trigger="on-event" event-name="test-event" scroll="true" href="/advanced_behaviors/dispatch_event/dispatch_event_append.xml" style="Main">
  <text style="Description">Dispatch events to load screens</text>
  <view style="Button" href="/advanced_behaviors/dispatch_event/dispatch_event_source.xml">
  <text style="Button__Label">Open new screen</text>
</view>
</view>

This view will listen for on-event trigger specifically for the event-name test-event (throws an error if event-name is not specified). When this event is caught, it does the append action and appends the XML fragment dispatch_event_append.xml which looks like:

<text xmlns="https://hyperview.org/hyperview">Let it Be!</text>

The source of this event is a different screen:

<view style="Button">
  <behavior trigger="press" action="dispatch-event" event-name="test-event" />
  <text style="Button__Label">Reload previous</text>
</view>

With the press trigger the dispatch-event action will fire an event (internal to hyperview) with the name defined by the event-name attribute (if not provided throws an error).

Overview

This uses an event emitter (or pub-sub) based approach for implementation. I went with this approach since the hyperview screens could be different for different routes (and navigators). The alternative approach which I didn't go with was to use the context API due to following reasons:

  1. Context is best used for sharing state via a global tree between different react components (which have to be a child of the context provider). It didn't make much sense to me to implement an event based pub/sub system using context.
  2. This will require us to wrap the root app navigator with a hyperview provider so all the screens can access the event dispatcher from the context consumer:
import { Provider } from 'hyperview';

...
<Provider>
  <AppNavigator />
</Provider>

An event emitter based approach looked better and simpler to me. I also tried to look into options if I could build this on top of react-navigation with some shared event publishers and subscribers but didn't spend much time on it since I wasn't able to find something related on the topic which I could use.

Also supports once and delay:

once

  • When added to a listener behavior (with trigger="on-event"), the behavior would only respond to the dispatched event once (till the screen is unmounted)
  • When added to the dispatching behavior (with action="dispatch-event"), the behavior will only dispatch the event once

delay

  • When added to a listener behavior (with trigger="on-event"), the behavior would respond to the dispatched event after the specified delay. (Note that when the delay is large this can lead to indeterminate UI state related)
  • When added to the dispatching behavior (with action="dispatch-event"), the behavior will dispatch the event after the specified delay

ToDo

  • Should I add a check for the presence of both trigger="on-event" and action="dispatch-event" and throw an error or warning?
  • Simplifying debugging around events. What should be logged in __DEV__ mode?
  • Avoiding event chaining? I think preventing adding both trigger="on-event" and action="dispatch-event" should be help with this but any better ideas?

Any other feedback? Would love some help with testing it for more actions/triggers in case there are any issues.

Demo

The following demos use the same example mentioned in the spec above. A screen is able to trigger an append in a different screen using the event dispatch system.

With prepend action With append action
demo prepend demo append

Debugging

Screen Shot 2019-07-03 at 3 31 44 PM

Next steps

  1. Add docs
  2. New release
  3. Test on Instawork app
  4. Blog post
@adamstep

This comment has been minimized.

Copy link
Contributor

adamstep commented Jul 3, 2019

for debugging:

  • log any dispatched events (along with string of dispatching element)
  • log any elements triggered by event (along with string of triggered element)
@adamstep

This comment has been minimized.

Copy link
Contributor

adamstep commented Jul 3, 2019

  • make sure to support behavior attributes of once and delay
divyanshu013 added 2 commits Jul 4, 2019
@divyanshu013

This comment has been minimized.

Copy link
Member Author

divyanshu013 commented Jul 4, 2019

Thanks for the feedback @adamstep, I've added support for once and delay as well.

@divyanshu013 divyanshu013 marked this pull request as ready for review Jul 4, 2019
Copy link
Contributor

flochtililoch left a comment

Looks good to me, thanks for implementing this!

@divyanshu013 divyanshu013 merged commit 25e589f into Instawork:master Jul 30, 2019
2 of 5 checks passed
2 of 5 checks passed
Header rules - upbeat-shaw-3c738f No header rules processed
Details
Pages changed - upbeat-shaw-3c738f All files already uploaded
Details
Redirect rules - upbeat-shaw-3c738f No redirect rules processed
Details
Mixed content - upbeat-shaw-3c738f No mixed content detected
Details
netlify/upbeat-shaw-3c738f/deploy-preview Deploy preview ready!
Details
@divyanshu013 divyanshu013 deleted the divyanshu013:element-dependency branch Jul 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.