Clean up stage persistence UI further

client/editor.js

@@ -17,15 +17,15 @@ const changeGlyph = (method, argument) => {
 const constructStage = (type) => {
   const glyph = Session.get('editor.glyph');
   stage = new stages[type](glyph);
+  stage.refreshUI(glyph.character, glyph.metadata);
+  glyph.stages[stage.type] = stage.getStageOutput();
   Session.set('editor.glyph', glyph);
-  stage._type = type;
-  stage.refresh(glyph);
 }
 
 this.getGlyph = (selector) => changeGlyph('getGlyph', selector);
 
 const incrementStage = (amount) => {
-  const index = types.indexOf(stage._type);
+  const index = types.indexOf(stage.type);
   if (index < 0) return;
   const new_index = index + amount;
   if (new_index < 0 || new_index >= types.length) return;
@@ -52,8 +52,9 @@ const bindings = {
 Template.editor.events({
   'click svg .selectable': function(event) {
     // We avoid the arrow function here so that this is bound to the template.
+    stage.handleEvent(event, this);
     const glyph = Session.get('editor.glyph');
-    stage.handleEvent(glyph, event, this);
+    glyph.stages[stage.type] = stage.getStageOutput();
     Session.set('editor.glyph', glyph);
   }
 });
@@ -78,7 +79,7 @@ Tracker.autorun(() => {
     types.map((x) => { if (glyph.stages[x]) last_completed_stage = x; });
     constructStage(last_completed_stage);
   }
-  stage.refresh(glyph);
+  stage.refreshUI(glyph.character, glyph.metadata);
   last_glyph = glyph;
 });
 

client/lib/abstract.js

@@ -1,39 +1,56 @@
 if (this.stages !== undefined) throw new Error('Redifining stages global!');
 this.stages = {};
 
+// Each stage is supposed to compute a particular field for the glyph.
+// It computes an initial value for this field based only on previous stages,
+// then exposes a UI for manual correction of its output.
+//
+// NOTE: No stage methods should update the glyph. The framework will do so by
+// calling getStageOutput when appropriate.
 stages.AbstractStage = class AbstractStage {
-  // This method should fill in this stage's field in glyph.stages. The glyph
+  // Initialize this stage's values based only off previous stages. Then, if the
-  // may already have a value for this stage set. If so, this stage's internal
+  // glyph already has a value for this stage's field and it is possible to set
-  // state should be initialized in such a way to achieve that output, if that
+  // up the internal state of this stage to achieve that value, set that state.
-  // is possible; doing so allows users to make some edits, switch to another
+  // This piece allows the user to resume editing a glyph.
-  // glyph, and then switch back and continue where they left off.
+  //
+  // Typically, a stage will maintain a 'this.original' variable containing the
+  // value without any manual edits and a 'this.adjusted' variable containing
+  // the value with manual edits.
   constructor(glyph) {
+    // The super constructor should be passed a type, but subclass constructors
+    // will be passed a glyph instead, hence the variable name discrepancy.
+    this.type = glyph;
+    this.colors = ['#0074D9', '#2ECC40', '#FFDC00', '#FF4136', '#7FDBFF',
+                   '#001F3F', '#39CCCC', '#3D9970', '#01FF70', '#FF851B'];
     // Session variables the interface by which the stage interacts with UI:
     //   - type - String type of this stage.
     //   - paths - list of dicts with keys in [cls, d, fill, stroke].
     //   - lines - list of dicts with keys in [cls, stroke, x1, y1, x2, y2].
     //   - points - list of dicts with keys in [cls, cx, cy, fill, stroke].
-    //   - instructions - String instructions for the user
     //   - status - list of dicts with keys in [cls, message] to log.
     //
     // The class name 'selectable' is special for paths, lines, and points.
     // Including this class in cls for those objects will make them interactive
     // and will trigger the onClick callback when they are clicked.
-    Session.set('stage.type', glyph);
+    Session.set('stage.type', this.type);
     Session.set('stage.paths', undefined);
     Session.set('stage.lines', undefined);
     Session.set('stage.points', undefined);
     Session.set('stage.status', undefined);
-    this.colors = ['#0074D9', '#2ECC40', '#FFDC00', '#FF4136', '#7FDBFF',
-                   '#001F3F', '#39CCCC', '#3D9970', '#01FF70', '#FF851B'];
   }
-  // Update the stage's internal state and possibly update this stage's field
+  // Return this stage's value based on current internal state. The default
-  // in glyph.stages based on the event.
+  // implementation works for stages that follow the 'original/adjusted'
-  handleEvent(glyph, event, template) {
+  // convention described in the constructor.
+  getStageOutput() {
+    return this.adjusted;
+  }
+  // Update the stage's internal state based on the event.
+  handleEvent(event, template) {
     assert(false, 'handleEvent was not implemented!');
   }
-  // Refresh the stage UI based on the current state of this stage.
+  // Refresh the stage UI based on the current state of this stage and the
-  refresh(glyph) {
+  // glyph's character and current metadata.
+  refreshUI(character, metadata) {
     assert(false, 'refresh was not implemented!');
   }
 }

client/lib/bridges.js

@@ -13,52 +13,50 @@ stages.bridges = class BridgesStage extends stages.AbstractStage {
   constructor(glyph) {
     super('bridges');
     const bridges = stroke_extractor.getBridges(glyph.stages.path);
-    this.bridges = bridges.bridges;
+    this.original = bridges.bridges;
-    this.endpoints = [];
+    this.adjusted = glyph.stages.bridges || this.original;
-    bridges.endpoints.map(
+    this.endpoints = bridges.endpoints.reduce((x, y) => x.concat(y), []);
-        (path) => this.endpoints = this.endpoints.concat(path));
+    this.path = glyph.stages.path;
     this.selected_point = undefined;
-    glyph.stages.bridges = glyph.stages.bridges || this.bridges;
   }
-  handleClickOnBridge(glyph, bridge) {
+  handleClickOnBridge(bridge) {
-    glyph.stages.bridges = removeBridge(glyph.stages.bridges, bridge);
+    this.adjusted = removeBridge(this.adjusted, bridge);
   }
-  handleClickOnPoint(glyph, point) {
+  handleClickOnPoint(point) {
     if (this.selected_point === undefined) {
       this.selected_point = point;
-      this.refresh(glyph);
+      this.refreshUI();
       return;
     } else if (Point.equal(point, this.selected_point)) {
       this.selected_point = undefined;
-      this.refresh(glyph);
+      this.refreshUI();
       return;
     }
     const bridge = [point, this.selected_point];
     this.selected_point = undefined;
-    const without = removeBridge(glyph.stages.bridges, bridge);
+    const without = removeBridge(this.adjusted, bridge);
-    if (without.length < glyph.stages.bridges.length) {
+    if (without.length < this.adjusted.length) {
-      this.refresh(glyph);
+      this.refreshUI();
       return;
     }
-    glyph.stages.bridges.push(bridge);
+    this.adjusted.push(bridge);
   }
-  handleEvent(glyph, event, template) {
+  handleEvent(event, template) {
     if (template.x1 !== undefined) {
-      this.handleClickOnBridge(
+      const bridge = [[template.x1, template.y1], [template.x2, template.y2]];
-          glyph, [[template.x1, template.y1], [template.x2, template.y2]]);
+      this.handleClickOnBridge(bridge);
     } else if (template.cx !== undefined) {
-      this.handleClickOnPoint(glyph, [template.cx, template.cy]);
+      this.handleClickOnPoint([template.cx, template.cy]);
     }
   }
-  refresh(glyph) {
+  refreshUI() {
-    Session.set('stage.paths',
+    Session.set('stage.paths', [{d: this.path, fill: 'gray', stroke: 'gray'}]);
-                [{d: glyph.stages.path, fill: 'gray', stroke: 'gray'}]);
     const keys = {};
-    this.bridges.map((bridge) => {
+    this.original.map((bridge) => {
       keys[bridgeKey(bridge)] = true;
       keys[bridgeKey(bridge.reverse())] = true;
     });
-    Session.set('stage.lines', glyph.stages.bridges.map((bridge) => ({
+    Session.set('stage.lines', this.adjusted.map((bridge) => ({
       cls: 'selectable',
       stroke: keys[bridgeKey(bridge)] ? 'red' : 'purple',
       x1: bridge[0][0],
@@ -80,8 +78,7 @@ stages.bridges = class BridgesStage extends stages.AbstractStage {
         stroke: color,
       }
     }));
-    const strokes = stroke_extractor.getStrokes(
+    const strokes = stroke_extractor.getStrokes(this.path, this.adjusted);
-        glyph.stages.path, glyph.stages.bridges);
     const n = strokes.strokes.length;
     const message = `Extracted ${n} stroke${n == 1 ? '' : 's'}.`;
     Session.set('stage.status', strokes.log.concat([{message: message}]));

client/lib/path.js

@@ -3,9 +3,10 @@
 stages.path = class PathStage extends stages.AbstractStage {
   constructor(glyph) {
     super('path');
+    this.adjusted = glyph.stages.path;
   }
-  refresh(glyph) {
+  refreshUI() {
-    const d = glyph.stages.path;
+    const d = this.adjusted;
     Session.set('stage.paths', [{d: d, fill: 'gray', stroke: 'gray'}]);
     Session.set('stage.status',
                 d ? [] : [{cls: 'error', message: 'No path data.'}]);

client/lib/strokes.js

@@ -24,28 +24,27 @@ stages.strokes = class StrokesStage extends stages.AbstractStage {
   constructor(glyph) {
     super('strokes');
     const include = this.include = {};
-    this.strokes = stroke_extractor.getStrokes(
+    this.original = stroke_extractor.getStrokes(
         glyph.stages.path, glyph.stages.bridges).strokes;
-    this.strokes.map((stroke) => include[stroke] = true);
+    this.original.map((x) => this.include[x] = true);
-    if (glyph.stages.strokes && glyph.stages.strokes.length > 0 &&
+    if (glyph.stages.strokes &&
         glyph.stages.strokes.filter((x) => !include[x]).length === 0) {
-      this.strokes.map((stroke) => include[stroke] = false);
+      this.original.map((x) => this.include[x] = false);
-      glyph.stages.strokes.map((stroke) => include[stroke] = true);
+      glyph.stages.strokes.map((x) => include[x] = true);
     }
-    glyph.stages.strokes = this.strokes.filter((x) => this.include[x]);
+    this.adjusted = this.original.filter((x) => this.include[x]);
   }
-  handleEvent(glyph, event, template) {
+  handleEvent(event, template) {
     assert(this.include.hasOwnProperty(template.d));
     this.include[template.d] = !this.include[template.d];
-    glyph.stages.strokes = this.strokes.filter((x) => this.include[x]);
+    this.adjusted = this.original.filter((x) => this.include[x]);
-    Session.set('editor.glyph', glyph);
   }
-  refresh(glyph) {
+  refreshUI(character, metadata) {
     Session.set('stage.paths',
-                getStrokePaths(this.strokes, this.include, this.colors));
+                getStrokePaths(this.original, this.include, this.colors));
-    const data = cjklib.getCharacterData(glyph.character);
+    const data = cjklib.getCharacterData(character);
-    const actual = glyph.stages.strokes.length;
+    const actual = this.adjusted.length;
-    const expected = glyph.metadata.strokes || data.strokes;
+    const expected = metadata.strokes || data.strokes;
     Session.set('stage.status', [getStatusLine(actual, expected)]);
   }
 }