Skip to content

Commit

Permalink
Problem: PR #60 introduced unintended regression - any call to _notif…
Browse files Browse the repository at this point in the history
…yPath resets an unsaved explicit composition created using CompositionEditor

Solution: change the _compositionChanged method to more clearly implement the complex process of selecting which composition to apply. Fix explicit composition caching so it does not get overwritten by fallback composition. Add a test
  • Loading branch information
warpech committed Oct 23, 2017
1 parent c4ce900 commit dc6c23a
Show file tree
Hide file tree
Showing 2 changed files with 68 additions and 21 deletions.
66 changes: 45 additions & 21 deletions starcounter-include.html
Original file line number Diff line number Diff line change
Expand Up @@ -210,35 +210,59 @@
this.template.model = this.partial
}
};

/**
* Stamps composition if needed
* @param {String} compositionString HTML as a string to compare if a re-render is needed
* @param {Function} callb Function that returns a composition to render
*/
StarcounterIncludePrototype._renderCompositionChange = function(compositionString, callb) {
if (!this._forceLayoutChange && this._lastCompositionString == compositionString) {
return;
}
let composition = callb();
this.stampComposition(composition);
this._forceLayoutChange = false;
this._lastCompositionString = compositionString;
}

/**
* Handles change of the composition.
* If not given explicitely, fetches one from provider.
* Stamps if needed.
* If not given in provider, fetches one from imported template's default composition.
* If imported template does not have a default composition, uses fallback composition.
* Warning: the explicit composition might come from https://github.com/Starcounter/starcounter-layout-html-editor/blob/17b21f729facd9a8dcd4241fb5c48cb71de11af5/starcounter-layout-html-editor.html#L253
* @see .stampComposition
* @param {String} compositionString stringified HTML for new Shadow Root
*/
StarcounterIncludePrototype._compositionChanged = function(compositionString) {
let composition;
if (this.partial && compositionString === undefined) {
const compositionProvider = findCompositionProvider(this.partial);
if (compositionProvider) {
compositionString = compositionProvider.Composition$; //should always be string
this.storedLayout = compositionString;
}
}
// do not change if there is one, it's same as before, and we are not forced
if (!compositionString) {
composition = this.defaultComposition && this.defaultComposition.cloneNode(true);
compositionString = this.defaultComposition;
} else {
// make clone to avoid direct binding
composition = this.stringToDocumentFragment(compositionString);
}
if (!compositionString || compositionString !== this._lastLayout || this._forceLayoutChange) {
this.stampComposition(composition);
this._forceLayoutChange = false;
this._lastLayout = compositionString;
var that = this;

if (compositionString !== undefined) {
this._forceLayoutChange = true;
this._renderCompositionChange("explicit or fallback", function() {
return that.stringToDocumentFragment(compositionString);
});
}
else {
const compositionProvider = this.partial && findCompositionProvider(this.partial);
if (compositionProvider && compositionProvider.Composition$) {
this._renderCompositionChange(compositionProvider.Composition$, function() {
that.storedLayout = compositionProvider.Composition$; //should always be string
return that.stringToDocumentFragment(that.storedLayout);
});
}
else if (this.defaultComposition) {
this._renderCompositionChange("imported template's default", function() {
return that.defaultComposition.cloneNode(true);
});
}
else {
this._renderCompositionChange("explicit or fallback", function() {
return undefined;
});
}
}
};
/**
* Forward Polymer notification downwards from `<template is="dom-bind">`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,29 @@
});
});

describe('after `dom-bind` changes the sub-partial property of a view with an explicit composition', function () {
var scope, include, newVal = "yeah", customComposition = "my custom composition";

beforeEach(function(done) {
sinon.stub(importedTemplate, "_notifyPath");
importedTemplate.addEventListener('stamping', function onceStamped(){
importedTemplate.removeEventListener('stamping', onceStamped);
include = container.querySelector('starcounter-include');
include._compositionChanged(customComposition); //this is how starcounter-layout-html-editor uses it
domBind.set('model.Page.scope1.doesItWork', newVal);
done();
});
});

it('should forward the notification to `imported-template`', function () {
expect(importedTemplate._notifyPath).to.be.calledWith("model.scope1.doesItWork", newVal);
});

it('should be preserve the explicit composition', function () {
expect(include.shadowRoot.innerHTML).to.be.equal(customComposition);
});
});

describe('after `dom-bind`` changed the sub-partial property (just one scope) to new one with thethat results with the same merged Html', function () {
var changedScopeWithSameHtml;
beforeEach(function(done) {
Expand Down

0 comments on commit dc6c23a

Please sign in to comment.