googlearchive/Propel

Add an event for state changes

Closed this issue · 13 comments

I had a rework of the Simple Push demo and thought this might be useful in terms of what an API could look like for the frontend client.

The Push Client Code (i.e. the code abstracted away into a library) is here:

https://github.com/gauntface/simple-push-demo/blob/master/app/scripts/push-client.es6.js

The use of it is here:

https://github.com/gauntface/simple-push-demo/blob/master/app/scripts/main.es6.js

The important parts are:

var pushClient = new PushClient(
  stateChangeListener,
  subscriptionUpdate
);

stateChangeListener is called when the UI for the web page needs to change. subscriptionUpdate is called when a subscription is found and should be sent to the application server.

The state change data is

var stateChangeListener = function(state, data) {
  // console.log(state);
  if (typeof(state.interactive) !== 'undefined') {
    if (state.interactive) {
      pushToggleSwitch.enable();
    } else {
      pushToggleSwitch.disable();
    }
  }

  if (typeof(state.pushEnabled) !== 'undefined') {
    if (state.pushEnabled) {
      pushToggleSwitch.on();
    } else {
      pushToggleSwitch.off();
    }
  }

  switch (state.id) {
  case 'ERROR':
    console.error(data);
    showErrorMessage(
      'Ooops a Problem Occurred',
      data
    );
    break;
  default:
    break;
  }
};

The state contains interactive and pushEnabled variables which indicate where the user should be able to interact with the UI - i.e. don't allow them to attempt to subscribe several times while we want to wait for permissions and pushEnabled is whether the UI should act as thought push has been enabled or not.

This covers a lot of use cases here. The data can be anything relevant and at the moment isn't used here when if should for (for examples errors).

This may change when it comes to enabling encryption however.

Apologies for my lack of understanding of how the various bits all fit together. Is the idea that the code in https://github.com/gauntface/simple-push-demo/blob/master/app/scripts/push-client.es6.js would be refactored out of gauntface/simple-push-demo and released as a client-side library as part of Propel?

No, I only raised this issue to open up discussion of how the client side api for propel could look for helping with state changed (I think my implementation could be simplified and be vastly improved).

Last time I checked propel had no way of helping developers change the state of their UI throughout the lifecycle of permission and subscription.

So, my read on this is that you want us to have some event listener for changes to the state of push messaging in the client. I agree that it makes sense, and I'm changing the issue title.

Discussing this with some people, would firing custom events be the best API for this?

pushClient.addEventListener('pushStateChange', function(event) {
    // event.isPushSubscribed
    // event.enableUI
});

There was some discussion of using a promise to manage the state but not sure what that would look like, is there a canonical source of what the library usage would be?

Yes, the state change should trigger a CustomEvent. I'm thinking that you will have something like:

pushClient.addEventListener('stateChange', function(event) {
  if (event.state === pushClient.STATE_SUBSCRIBED) {
    ...
  }
});

though obviously we can bike-shed the details forever :)

There are a few different ways change of state flow could be tackled here (including Promises or Observables) but custom events may just be the simplest. Perhaps consider shipping with support for that and iterating later on based on developer feedback?

Promises I can't see how it would work the more I think about it.

Would the state list be:

pushClient.addEventListener('stateChange', function(event) {
  switch (event.state === pushClient.STATE_SUBSCRIBED) {
    case pushClient.STATE_UNSUBSCRIBED:
      // TODO: show UI for push not subscribed
      break;
    case pushClient.STATE_SUBSCRIBED:
      // TODO: show UI for push subscribed
      break;
    case pushClient.STATE_PERMISSION_BLOCKED:
      // TODO: disable - perhaps show message to user
      break;
    case pushClient.STATE_NOT_SUPPORTED:
    default:
      // TODO: disable or hide UI
      break;
  }
});

The only thing that this doesn't help developers with is disabling the UI elements, but the library can leave that to the developer if we think that is best.

Note for Mat - Use permission API to get an event when the user disables push (something the notification API doesn't support)

Also note for me: Make sure that there is an event fired that is an appropriate alternative to #3.

Apologies I didn't read all the details of discussion here, but one quick comment is that personally I find having both an event-based interface and a promise-based interface useful, and if we support event-based it should be possible to add as many listeners as you like, rather than just one as was the design in Matt's SimplePush example.

I ended up modifying the library to use an addStateChangeListener pattern instead as I needed to listen to state changes in some networking code and UI.

One comment from the events PR (4332a78#commitcomment-15685019) is that we fire a subscriptionUpdate for every page load even though the subscription object hasn't changed.

We should use indexDB to detect changes to subscription to make firing unsubscribe and subscriptionchange events a little smarter.

This seems to be addressed in the current state of Propel so closing this.