diff --git a/packages/spark/spark.js b/packages/spark/spark.js index ab1820e82d..cc1a2f5272 100644 --- a/packages/spark/spark.js +++ b/packages/spark/spark.js @@ -14,6 +14,10 @@ // XXX if not on IE6-8, don't do the extra work (traversals for event // setup) those browsers require +// XXX flag errors if you have two landmarks with the same branch +// path, or if you have multiple preserve nodes in a landmark with the +// same selector and label + (function() { Spark = {}; @@ -79,6 +83,9 @@ Spark._Renderer = function (getLandmarkState) { // happening (XXX document better) this.labelStack = [this.landmarkTree]; + // All landmark ranges created during this rendering. + this.landmarks = []; + // Function to call when a landmark is encountered during rendering // (that is, when createLandmark is called) to get the state object // for that landmark (from a previous version of the landmark.) Or @@ -221,9 +228,13 @@ var materialize = function (html, renderer) { // newly rendered fragment must be on the screen (if it doesn't want // to get garbage-collected.) // +// 'landmarks' is a list of the landmark ranges in 'frag'. It may be +// omitted if frag doesn't contain any landmarks. +// // XXX expose in the public API, eg as Spark.introduce(), so the user -// can call it when manually inserting nodes? (via, eg, jQuery?) -var scheduleOnscreenSetup = function (frag) { +// can call it when manually inserting nodes? (via, eg, jQuery?) -- of +// course in that case 'landmarks' would be empty. +var scheduleOnscreenSetup = function (frag, landmarks) { var renderedRange = new LiveRange(Spark._TAG, frag); var finalized = false; renderedRange.finalize = function () { @@ -258,15 +269,19 @@ var scheduleOnscreenSetup = function (frag) { } } + // Deliver render callbacks to all landmarks that are now + // onscreen (possibly not for the first time.) + // + // XXX should bubble up and notify parent landmarks too? for all + // the same reasons we need to do it for node preservation? + _.each(landmarks, function (landmark) { + landmark.renderCallback.call(landmark.state); + }); + // This code can run several times on the same nodes (if the // output of a render is included in a render), so it must be // idempotent. This is not the best, asymptotically. There are - // things we could do to improve it, like leaving renderedRange in - // place and making notifyLandmarksRendered skip its contents (but - // this would require that we adjust isolate() -- see comment - // there about junk ranges), or letting each landmark schedule its - // own onscreen processing. - notifyLandmarksRendered(renderedRange); + // things we could do to improve it. notifyWatchers(renderedRange.firstNode(), renderedRange.lastNode()); renderedRange.destroy(); }); @@ -279,7 +294,7 @@ Spark.render = function (htmlFunc) { var html = Spark._currentRenderer.withValue(renderer, htmlFunc); var frag = materialize(html, renderer); - scheduleOnscreenSetup(frag); + scheduleOnscreenSetup(frag, renderer.landmarks); return frag; }; @@ -385,11 +400,8 @@ _.extend(PreservationController.prototype, { } }); -// Modify `range` so that it matches the result of -// Spark.render(htmlFunc). `range` must be in `document` (that is, -// onscreen.) If the old contents had any landmarks that match -// landmarks in `frag`, move the landmarks over and perform any node -// or region preservations that they request. +// `range` is a region of `document`. Modify it in-place so that it +// matches the result of Spark.render(htmlFunc), preserving landmarks. Spark.renderToRange = function (range, htmlFunc) { var getLandmarkState = function () { var node = renderer.labelStack[this.labelStack.length - 1]; @@ -409,7 +421,7 @@ Spark.renderToRange = function (range, htmlFunc) { var html = Spark._currentRenderer.withValue(renderer, htmlFunc); var frag = materialize(html, renderer); - scheduleOnscreenSetup(frag); + scheduleOnscreenSetup(frag, renderer.landmarks); DomUtils.wrapFragmentForContainer(frag, range.containerNode()); @@ -436,7 +448,7 @@ Spark.renderToRange = function (range, htmlFunc) { pc.addRoot(walk.containerNode(), walk.preserve, range, tempRange); - // compute preservations (must do this before destorying tempRange) + // compute preservations (must do this before destroying tempRange) var preservations = pc.computePreservations(range, tempRange); tempRange.destroy(); @@ -844,16 +856,20 @@ Spark.createLandmark = withRenderer(function (options, html, _renderer) { top.current = state; return _renderer.annotate( - html, Spark._ANNOTATION_LANDMARK, { - preserve: preserve, - constant: !! options.constant, - renderCallback: options.render || function () {}, - destroyCallback: options.destroy || function () {}, - state: state, - finalize: function () { - if (! this.superceded) - this.destroyCallback.call(this.state); - } + html, Spark._ANNOTATION_LANDMARK, function (range) { + _.extend(range, { + preserve: preserve, + constant: !! options.constant, + renderCallback: options.render || function () {}, + destroyCallback: options.destroy || function () {}, + state: state, + finalize: function () { + if (! this.superceded) + this.destroyCallback.call(this.state); + } + }); + + _renderer.landmarks.push(range); }); // XXX need to arrange for destroyCallback to be called if the @@ -901,24 +917,4 @@ var visitLandmarkTree = function (tree, range, func) { }); }; -// Find all the landmarks in `range` and let them know that they are -// now onscreen. If it's their first time being onscreen, they need to -// have their `create` callback called. And they need `render` whether -// it is their first time or not. Idempotent. -var notifyLandmarksRendered = function (range) { - range.visit(function (isStart, r) { - if (isStart && r.type == Spark._ANNOTATION_LANDMARK) { - if (!r.rendered) { - // XXX should be render(start, end) ?? - - // XXX should bubble up and notify parent landmarks too? for - // all the same reasons we need to do it for node - // preservation? - r.renderCallback.call(r.state); - r.rendered = true; - } - } - }); -}; - })(); \ No newline at end of file diff --git a/packages/spark/spark_tests.js b/packages/spark/spark_tests.js index 338ebc1a48..f54d444e5f 100644 --- a/packages/spark/spark_tests.js +++ b/packages/spark/spark_tests.js @@ -1215,20 +1215,20 @@ Tinytest.add("spark - labeled landmarks", function (test) { expect(["c", 1, "c", 2, "c", 5, "c", 4, "c", 3], [1, 2, 5, 4, 3]); Meteor.flush(); - expect(["r", 1, "r", 2, "r", 3, "r", 4, "r", 5], [1, 2, 3, 4, 5]); + expect(["r", 1, "r", 2, "r", 5, "r", 4, "r", 3], [1, 2, 5, 4, 3]); for (var i = 0; i < 10; i++) { R[i].set(1); expect([], []); Meteor.flush(); - expect(["r", 1, "r", 2, "r", 3, "r", 4, "r", 5], - [i*5 + 6, i*5 + 7, i*5 + 8, i*5 + 9, i*5 + 10]); + expect(["r", 1, "r", 2, "r", 5, "r", 4, "r", 3], + [i*5 + 6, i*5 + 7, i*5 + 10, i*5 + 9, i*5 + 8]); }; excludeLandmarks[2].set(true); expect([], []); Meteor.flush(); - expect(["d", 2, "r", 1, "r", 3, "r", 4, "r", 5], - [52, 56, 57, 58, 59]); + expect(["d", 2, "r", 1, "r", 5, "r", 4, "r", 3], + [52, 56, 59, 58, 57]); excludeLandmarks[2].set(false); excludeLandmarks[3].set(true); @@ -1241,32 +1241,32 @@ Tinytest.add("spark - labeled landmarks", function (test) { excludeLandmarks[3].set(false); expect([], []); Meteor.flush(); - expect(["c", 5, "c", 4, "c", 3, "d", 2, "r", 1, "r", 3, "r", 4, "r", 5], - [65, 64, 63, 61, 62, 63, 64, 65]); + expect(["c", 5, "c", 4, "c", 3, "d", 2, "r", 1, "r", 5, "r", 4, "r", 3], + [65, 64, 63, 61, 62, 65, 64, 63]); excludeLandmarks[2].set(false); expect([], []); Meteor.flush(); - expect(["c", 2, "r", 1, "r", 2, "r", 3, "r", 4, "r", 5], - [67, 66, 67, 68, 69, 70]); + expect(["c", 2, "r", 1, "r", 2, "r", 5, "r", 4, "r", 3], + [67, 66, 67, 70, 69, 68]); isolateLandmarks.set(true); expect([], []); Meteor.flush(); - expect(["r", 1, "r", 2, "r", 3, "r", 4, "r", 5], - [71, 72, 73, 74, 75]); + expect(["r", 1, "r", 2, "r", 5, "r", 4, "r", 3], + [71, 72, 75, 74, 73]); for (var i = 0; i < 10; i++) { var expected = [ - [["r", 1, "r", 2, "r", 3, "r", 4, "r", 5], [76, 77, 78, 79, 80]], + [["r", 1, "r", 2, "r", 5, "r", 4, "r", 3], [76, 77, 80, 79, 78]], [["r", 1], [81]], - [["r", 1, "r", 2, "r", 3, "r", 4, "r", 5], [82, 83, 84, 85, 86]], + [["r", 1, "r", 2, "r", 5, "r", 4, "r", 3], [82, 83, 86, 85, 84]], [["r", 2], [87]], - [["r", 1, "r", 2, "r", 3, "r", 4, "r", 5], [88, 89, 90, 91, 92]], - [["r", 3, "r", 4, "r", 5], [93, 94, 95]], - [["r", 3, "r", 4, "r", 5], [96, 97, 98]], - [["r", 4, "r", 5], [99, 100]], - [["r", 4, "r", 5], [101, 102]], + [["r", 1, "r", 2, "r", 5, "r", 4, "r", 3], [88, 89, 92, 91, 90]], + [["r", 5, "r", 4, "r", 3], [95, 94, 93]], + [["r", 5, "r", 4, "r", 3], [98, 97, 96]], + [["r", 5, "r", 4], [100, 99]], + [["r", 5, "r", 4], [102, 101]], [["r", 5], [103]] ][i]; R[i].set(2); @@ -1282,11 +1282,11 @@ Tinytest.add("spark - labeled landmarks", function (test) { excludeLandmarks[4].set(false); excludeLandmarks[5].set(true); Meteor.flush(); - expect(["c", 4, "r", 3, "r", 4], [106, 105, 106]); + expect(["c", 4, "r", 4, "r", 3], [106, 106, 105]); excludeLandmarks[5].set(false); Meteor.flush(); - expect(["c", 5, "r", 4, "r", 5], [108, 107, 108]); + expect(["c", 5, "r", 5, "r", 4], [108, 108, 107]); }); @@ -2671,12 +2671,12 @@ Tinytest.add("spark - oldschool landmark matching", function(test) { Meteor.flush(); // what order of chunks {0,1} is preferable?? // should be consistent but I'm not sure what makes most sense. - test.equal(buf, "c1,c0,r0,r1".split(',')); + test.equal(buf, "c1,c0,r1,r0".split(',')); buf.length = 0; R.set("B"); Meteor.flush(); - test.equal(buf, "r0,r1".split(',')); + test.equal(buf, "r1,r0".split(',')); buf.length = 0; div.kill();