UX - Are you sure you want to discard your drafts?

[mike-north]

This is a nice UX pattern that goes along with many of this addon's use cases.

window.onbeforeunload =  function() {
  if (hasDrafts) {
    var message = "Are you sure you want to lose all your drafts?";
    if (confirm(message)) return true;
    else return false;
  }
}

The info we need here in order to provide a nice API surface for the simplest use case would be:

• states to check for the presence of "draft data" on
• property key(s) of the draft state properties in question
• some indication as to the keys to use in the weak map for the given state name

A practical example

• post is an ember-data model in our app
• comment-draft is the name of our state
• body is the "important" property key on the comment-draft state

Let's just say "important" means something we need to ensure ensure is not thrown away (via reload/navigation/closing browser tab) without user consent.

In the component for our post, we'll of course have some UI element bound to commentDraft.body and wire up the state using stateFor as instructed.

   commentDraft: stateFor('comment-draft', 'post'),

What I as a developer would want to express via some API

Get the user's confirmation before the window is unloaded if, for any of the post models currently loaded in the store, any of the respective comment-draft states have a non-empty body value on them.

Open questions

• Is "non-empty" the only thing we usually need to check for?
• Is some level of customizability needed for the content of the confirmation message (i.e., on a per-type basis?)
• Currently there's no explicit reference to ember-data in ember-state-services. Is there a way to make it really easy for ember-data use cases (i.e., by just specifying a record type -- post to evaluate when making this check), while still providing the flexibility to make this useful broadly for any valid "key" object?

Not sure whether this would be best implemented as a new addon or as part of this addon, but this seems like the best place for the discussion

[thoov]

The way I have thought about doing something similar was to set your 'state bucket' to use buffered-proxies. This gives you the isolation properties of state-services with the staging and rollback properties of buffered-proxy.

From a highlevel it would look like this:

// app/states/comment-draft
import BufferedProxy from 'ember-buffered-proxy/proxy';

export default BufferedProxy.extend();

// app/controllers/foo
export default Ember.Controller.extend({
  commentDraft: stateFor('comment-draft', 'post'),
  // ...
});

// app/routes/foo
export default Ember.Route.extend({
  actions: {
    willTransition(transition) {
      // hasChanges comes from bufferedProxy
      if (this.controller.get('commentDraft.hasChanges') {
        alert('Oops! You still have changes');
        transition.abort();
      }
    }
  }
});

You mention body being the important field to check for. Currently in buffered proxy there is no way to check if a single property has changed since we only have hasChanged. (Unless you want to do something like buffer.get('body') === buffer.get('content.body'); This is something that we should probably add to buffered proxy to enable a better check like:

const commentDraft = this.controller.get('commentDraft');
commentDraft.hasChanged('body'); => true/false

As for the open questions:

Is some level of customizability needed for the content of the confirmation message (i.e., on a per-type basis?)

I'm not sure if this needs to be in the addon. I think its probably safe for end users to just roll this themselves or we can create a separate addon that overrides the willTransition with its own logic.

Currently there's no explicit reference to ember-data in ember-state-services. Is there a way to make it really easy for ember-data use cases (i.e., by just specifying a record type -- post to evaluate when making this check), while still providing the flexibility to make this useful broadly for any valid "key" object?

I will defer to @stefanpenner as I try to not deal with ember-data as much as possible :-)

[mike-north]

@thoov I like the pattern you've used here, and I think that this solves another (important) use case involving transitioning away from the route and resetting state back to "clean".

What I'm referring to here here is the ability to warn users that they may have something "dirty" or otherwise in "draft" form at the time they essentially try to leave the app via doing something like closing the tab. The key difference being that the user may be far away from the controller/route that deals with the state in question, so it seems like it may be an application-level issue?

[mike-north]

This is the super hacky "don't try this at home" thing I'm using for a demo. Works for any state objects, but ultimately uses DS.Store#peekAll to get all loaded records of a given type in order to access the appropriate states that I care about, and ultimately the properties on those states

Because no reference to state objects is created that might interfere w/ the "weakness" of the weakmap (the function for the hasImportantDrafts CP is a pure function), this shouldn't create any leaking.

Time complexity is

   (a) NUM_RECORD_TYPES_TO_WATCH
X  (b) NUM_LOADED_RECORDS_OF_THOSE_TYPES
X  (c) NUM_STATES_YOU_CARE_ABOUT_FOR THOSE_RECORDS
X  (d) NUM_PROPS_YOU_CARE_ABOUT_ON_THOSE_STATES

I would expect (a) (c) and (d) to be very small for most use cases

app/routes/application.js

import Ember from 'ember';

const { Route, inject } = Ember;

export default Route.extend({
  'draft-watcher': inject.service(),

  // Modern browsers won't display this for security/scamming reasons,
  //  but IE8-10 will show it
  confirmMessage: `You have unsaved drafts which will be lost if you leave.
Are you sure?`,

  activate() {
    this._super(...arguments);
    window.onbeforeunload = this._onBeforeUnload.bind(this);
  },

  _onBeforeUnload() {
    if (this.get('draft-watcher.hasImportantDrafts')) {
      // Seems that the important thing to do here is to return
      //  a value only if we wish to pop up the "Are you really sure?"
      return this.get('confirmMessage');
    }
  }
});

app/services/draft-watcher.js

import Ember from 'ember';
import stateFor from 'ember-state-services/state-for';

const {
  Service,
  inject,
  computed
} = Ember;

export default Service.extend({
  store: inject.service(),

  // ember-data record types to check
  recordTypesToCheck: {
    // for each record type to check, state types to check
    post: {
      // for each state type, properties to check (for emptiness)
      'post-info': ['body']
    }
  },

  /**
   * Check to see if a given state's property is to be considered "dirty"
   * For contextual usefulness, also give access to the name of the record
   * type, the state name, and the property key on the state object
   *
   * @private
   */
  _isStatePropertyDirty(recordType, stateType, propKey, propValue) {
    return !!propValue;
  },

  /**
   * Hacky way of using public APIs only to get access to the WeakMap
   * for a given state name. Can't create CPs on Ember.Object.create anymore
   *
   * @private
   */
  _weakMapForState(stateName) {
    let cachedSource = this.get(`_weakMapsForStates.${stateName}`);
    if (cachedSource) {
      return cachedSource;
    } else {
      // jscs:disable disallowDirectPropertyAccess
      let newSource = Ember.Object.extend({
        // jscs:enable disallowDirectPropertyAccess
        recordStateMap: stateFor(stateName)
      }).create().get('recordStateMap');
      this.set(`_weakMapsForStates.${stateName}`, newSource);
      return newSource;
    }
  },
  _weakMapsForStates: {},

  _statesForRecords(weakmap, records) {
    return records.map((r) => weakmap.get(r));
  },

  /**
   * A boolean (volatile) CP that tells us if any of our "important"
   * properties on certain states, for certain ember-data records are
   * non-empty
   *
   * @public
   */
  hasImportantDrafts: computed(function() {
    // Get all of the record types to check ( ['post'] )
    let recordTypesToCheck = this.get('recordTypesToCheck');
    // Iterate over them
    for (let typ in recordTypesToCheck) { // typ = 'post'
      // Get all of the loaded records of this type
      let records = this.get('store').peekAll(typ);
      // Get names of all of the states we're interested in checking
      for (let stateName in recordTypesToCheck[typ]) {
        // Get the weakmap for this state
        let recordStateMap = this._weakMapForState(stateName);
        // Transform the array of records to the corresponding array
        //   of states.
        let stateObjects = this._statesForRecords(recordStateMap, records);
        // Iterate over all state objects for this record
        for (let i = 0; i < stateObjects.length; i++) {
          let stateObj = stateObjects[i];
          // Iterate over all property keys we consider "important" on this state
          let statePropNames = recordTypesToCheck[typ][stateName];
          for (let j = 0; j < statePropNames.length; j++) {
            let statePropKey = statePropNames[j];
            // Finally, get the value of this state's property
            let statePropValue = stateObj.get(statePropKey);
            // Check to see if it's "dirty" or not
            if (this._isStatePropertyDirty(typ, stateName, statePropKey, statePropValue)) {
              // ...and early terminate this check as soon as we find anything
              return true;
            }
          }
        }
      }
    }
  }).volatile()
});

[stefanpenner]

small nitpick: hasImportantDrafts: computed(function() { /*...*/}).volatile() seems like its just a function no?

---

peekAll seems unfortunate, in the case of data models, it seems like we could keep a counter of "drafts" around, and increment/decrement at the appropriate time to keep track of if there are pending things or not.

[mike-north]

Yes, just a function. In my demo app there was usefulness in binding to the
value returned by the function
On Tue, Oct 25, 2016 at 7:52 PM Stefan Penner [email protected]
wrote:

small nitpick: hasImportantDrafts: computed(function() {
/.../}).volatile() seems like its just a function no?

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#64 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAiDtSKPat8uX0GaFgHF8-MJcBqWa6SNks5q3kHQgaJpZM4Ke-oV
.

[mike-north]

A self-nitpick here - Just realized that stateFor/1 (one argument) returns the weakmap and I don't have to use the CP on the cached objects. This is not documented in the README, but seems from code comments to be part of the public API.

@thoov confirm?