Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

FLUID-5282: protoComponent expander should respect floating model reference under new ChangeApplier model #478

Merged
merged 8 commits into from

3 participants

@amb26
Owner

@colinbdclark - Last one of this set for you!

Cindy Qi Li and others added some commits
@amb26 amb26 referenced this pull request in amb26/infusion
Closed

FLUID-5280: Added another unit test. #3

@cindyli
Owner

Thanks for fixing reported issues, @amb26. I tried this pull request with the metadata demo and the new relay system works beautifully.

@cindyli
Owner

@amb26, another issue with the new model relay system: issues.fluidproject.org/browse/FLUID-5285

I've issued a pull request amb26#4 against your FLUID-5282 branch https://github.com/amb26/infusion/tree/FLUID-5282 with an unit test to demonstrate this issue.

@cindyli
Owner

As per the discussion with Antranig in the irc channel - https://botbot.me/freenode/fluid-work/2014-03-11/?tz=America/Havana, 3:08PM onwards, the issue above was indeed caused by using "*" rather than "" as the path for the model listener. The FLUID-5282 jira and the pull request for it have been closed without a fix.

@colinbdclark colinbdclark merged commit 244bfce into fluid-project:master
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Feb 18, 2014
  1. Merge branch 'master' into FLUID-5279

    Cindy Qi Li authored
Commits on Feb 19, 2014
  1. @amb26

    FLUID-5279: Stopgap fix for FLUID-5279 (old renderer will be destroye…

    amb26 authored
    …d, no better is warranted)
  2. Merge branch 'master' into FLUID-5279-antranig

    Cindy Qi Li authored
  3. FLUID-5279: Added a unit test to demonstrate the issue that the defau…

    Cindy Qi Li authored
    …lt model value takes precedent over the relayed new model value in conditional renderers.
Commits on Feb 20, 2014
  1. @amb26
Commits on Feb 28, 2014
  1. FLUID-5280: Added another unit test to demonstrate an issue that the …

    Cindy Qi Li authored
    …default model value takes precedent over the relayed value when the change request is fired in component's onReady event.
Commits on Mar 5, 2014
  1. @amb26

    FLUID-5282: protoComponent expander should respect floating model ref…

    amb26 authored
    …erence under new ChangeApplier model
This page is out of date. Refresh to see the latest.
View
10 src/framework/core/js/DataBinding.js
@@ -309,6 +309,10 @@ var fluid_1_5 = fluid_1_5 || {};
});
};
+ fluid.sortCompleteLast = function (reca, recb) {
+ return (reca.completeOnInit ? 1 : 0) - (recb.completeOnInit ? 1 : 0);
+ };
+
// Operate all coordinated transactions by bringing models to their respective initial values, and then commit them all
fluid.operateInitialTransaction = function (instantiator, mrec) {
var transId = fluid.allocateGuid();
@@ -319,7 +323,8 @@ var fluid_1_5 = fluid_1_5 || {};
transRec[recel.that.applier.applierId] = {transaction: transac};
return transac;
});
- fluid.each(mrec, function (recel) {
+ var recs = fluid.values(mrec).sort(fluid.sortCompleteFirst);
+ fluid.each(recs, function (recel) {
var that = recel.that;
var transac = transacs[that.id];
if (recel.completeOnInit) {
@@ -1062,7 +1067,7 @@ var fluid_1_5 = fluid_1_5 || {};
};
fluid.initModelEvent = function (trans, listeners) {
- fluid.notifyModelChanges(listeners, "ADD", trans.newHolder, fluid.emptyHolder, {transactionId: trans.id});
+ fluid.notifyModelChanges(listeners, "ADD", trans.oldHolder, fluid.emptyHolder, {transactionId: trans.id});
};
fluid.emptyHolder = { model: undefined };
@@ -1128,6 +1133,7 @@ var fluid_1_5 = fluid_1_5 || {};
resolverGetConfig: options.resolverGetConfig
},
reset: function () {
+ trans.oldHolder = holder;
trans.newHolder = { model: fluid.copy(holder.model) };
trans.changeRecord.changes = 0;
trans.changeRecord.changeMap = {};
View
44 src/framework/renderer/js/RendererUtilities.js
@@ -67,6 +67,7 @@ fluid_1_5 = fluid_1_5 || {};
/** "Renderer component" infrastructure **/
// TODO: fix this up with IoC and improved handling of templateSource as well as better
// options layout (model appears in both rOpts and eOpts)
+ // "options" here is the original "rendererFnOptions"
fluid.renderer.createRendererSubcomponent = function (container, selectors, options, parentThat, fossils) {
options = options || {};
var source = options.templateSource ? options.templateSource : {node: $(container)};
@@ -81,12 +82,7 @@ fluid_1_5 = fluid_1_5 || {};
fluid.renderer.reverseMerge(rendererOptions, cascadeOptions, fluid.keys(cascadeOptions));
}
- var expanderOptions = fluid.renderer.modeliseOptions(options.expanderOptions, {ELstyle: "${}"}, parentThat);
- fluid.renderer.reverseMerge(expanderOptions, options, ["resolverGetConfig", "resolverSetConfig"]);
var that = {};
- if (!options.noexpand) {
- that.expander = fluid.renderer.makeProtoExpander(expanderOptions, parentThat);
- }
var templates = null;
that.render = function (tree) {
@@ -154,21 +150,39 @@ fluid_1_5 = fluid_1_5 || {};
});
fluid.rendererComponent.renderOnInit = function (renderOnInit, that) {
- if (renderOnInit) {
+ if (renderOnInit || that.renderOnInit) {
that.refreshView();
}
};
+
+ fluid.protoExpanderForComponent = function (parentThat, options) {
+ var expanderOptions = fluid.renderer.modeliseOptions(options.expanderOptions, {ELstyle: "${}"}, parentThat);
+ fluid.renderer.reverseMerge(expanderOptions, options, ["resolverGetConfig", "resolverSetConfig"]);
+ var expander = fluid.renderer.makeProtoExpander(expanderOptions, parentThat);
+ return expander;
+ };
fluid.rendererComponent.refreshView = function (that) {
- fluid.renderer.clearDecorators(that);
- that.events.prepareModelForRender.fire(that.model, that.applier, that);
- var tree = that.produceTree(that);
- if (that.renderer.expander) {
- tree = that.renderer.expander(tree);
+ if (!that.renderer) {
+ // Terrible stopgap fix for FLUID-5279 - all of this implementation will be swept away
+ // model relay may cause this to be called during init, and we have no proper definition for "that.renderer" since it is
+ // constructed in a terrible way
+ that.renderOnInit = true;
+ return;
+ } else {
+ fluid.renderer.clearDecorators(that);
+ that.events.prepareModelForRender.fire(that.model, that.applier, that);
+ var tree = that.produceTree(that);
+ var rendererFnOptions = that.renderer.rendererFnOptions;
+ // Terrible stopgap fix for FLUID-5821 - given that model reference may be rebound, generate the expander from scratch on every render
+ if (!rendererFnOptions.noexpand) {
+ var expander = fluid.protoExpanderForComponent(that, rendererFnOptions);
+ tree = expander(tree);
+ }
+ that.events.onRenderTree.fire(that, tree);
+ that.renderer.render(tree);
+ that.events.afterRender.fire(that);
}
- that.events.onRenderTree.fire(that, tree);
- that.renderer.render(tree);
- that.events.afterRender.fire(that);
};
fluid.rendererComponent.produceTree = function (that) {
@@ -219,9 +233,9 @@ fluid_1_5 = fluid_1_5 || {};
if (rendererFnOptions.rendererTargetSelector) {
container = function () {return that.dom.locate(rendererFnOptions.rendererTargetSelector); };
}
-
var renderer = {
fossils: {},
+ rendererFnOptions: rendererFnOptions,
boundPathForNode: function (node) {
return fluid.boundPathForNode(node, renderer.fossils);
}
View
13 src/tests/framework-tests/renderer/html/RendererUtilities-test.html
@@ -177,6 +177,19 @@ <h2 id="qunit-userAgent"></h2>
<span class="flc-fluid4986-simpleBound6"></span>
</div>
+ <div id="FLUID-5279">
+ <div class="flc-main"></div>
+ <div class="flc-sub"></div>
+ </div>
+
+ <div id="FLUID-5280">
+ <div class="flc-fluid5280-main"></div>
+ <div class="flc-fluid5280-sub"></div>
+ </div>
+
+ <div id="FLUID-5282">
+ </div>
+
</div>
<script>
View
151 src/tests/framework-tests/renderer/js/RendererUtilitiesTests.js
@@ -745,6 +745,155 @@ fluid.registerNamespace("fluid.tests");
jqUnit.assertDomEquals("Click handlers registered by afterRender", inputs, that.clicked);
});
+ // FLUID-5279: "that.produceTree is not a function" when refreshView() is called as a model (relayed) listener on a renderer relay component
+ fluid.defaults("fluid.tests.fluid5279", {
+ gradeNames: ["fluid.rendererRelayComponent", "autoInit"],
+ components: {
+ attributes: {
+ type: "fluid.rendererRelayComponent",
+ createOnEvent: "afterRender",
+ container: ".flc-sub",
+ options: {
+ model: "{fluid5279}.model",
+ modelListeners: {
+ "audio": "{that}.refreshView"
+ },
+ events: {
+ afterRender: "{fluid5279}.events.afterAttributesRendered"
+ },
+ resources: {
+ template: {
+ resourceText: "<div></div>"
+ }
+ }
+ }
+ }
+ },
+ model: {
+ audio: "available"
+ },
+ resources: {
+ template: {
+ resourceText: "<div></div>"
+ }
+ },
+ events: {
+ afterAttributesRendered: null,
+ onReady: {
+ events: {
+ onCreate: "onCreate",
+ afterAttributesRendered: "afterAttributesRendered"
+ },
+ args: "{that}"
+ }
+ },
+ listeners: {
+ "onCreate.init": "{that}.refreshView"
+ }
+ });
+
+ jqUnit.asyncTest("FLUID-5279: Direct model sharing for renderer relay components", function () {
+ var that = fluid.tests.fluid5279(".flc-main", {
+ listeners: {
+ onReady: function (that) {
+ jqUnit.assertNotUndefined("The component has been instantiated", that);
+ jqUnit.start();
+ }
+ }
+ });
+ });
+
+ // FLUID-5280: During initial transaction, give priority to recently modified values
+ fluid.defaults("fluid.tests.fluid5280", {
+ gradeNames: ["fluid.rendererRelayComponent", "autoInit"],
+ components: {
+ attributes: {
+ type: "fluid.tests.fluid5280sub",
+ createOnEvent: "afterRender",
+ container: ".flc-fluid5280-sub",
+ options: {
+ model: "{fluid5280}.model",
+ modelListeners: {
+ "audio": "{that}.refreshView"
+ },
+ resources: {
+ template: {
+ resourceText: "<div></div>"
+ }
+ }
+ }
+ }
+ },
+ model: {
+ audio: "available"
+ },
+ resources: {
+ template: {
+ resourceText: "<div></div>"
+ }
+ }
+ });
+
+ fluid.defaults("fluid.tests.fluid5280sub", {
+ gradeNames: ["fluid.rendererRelayComponent", "autoInit"],
+ protoTree: {
+ expander: {
+ "type": "fluid.renderer.condition",
+ "condition": {
+ "funcName": "fluid.tests.fluid5280.checkAudio",
+ "args": "${audio}"
+ }
+ }
+ },
+ model: {
+ audio: "available"
+ }
+ });
+
+ fluid.tests.fluid5280.newValue = "unavailable";
+
+ fluid.tests.fluid5280.checkAudio = function (audioValue) {
+ jqUnit.assertEquals("The relayed new value takes precedence over the default model value", fluid.tests.fluid5280.newValue, audioValue);
+ };
+
+ jqUnit.test("FLUID-5280: The relayed new value takes precedence over the default model value", function () {
+ var that = fluid.tests.fluid5280(".flc-fluid5280-main");
+ that.applier.requestChange("audio", fluid.tests.fluid5280.newValue);
+ that.refreshView();
+ });
+
+ // FLUID-5281: protoComponent expansion should respect new ChangeApplier idiom of "floating base model reference"
+
+ fluid.defaults("fluid.tests.fluid5282root", {
+ gradeNames: ["fluid.rendererRelayComponent", "autoInit"],
+ protoTree: {
+ expander: {
+ "type": "fluid.renderer.condition",
+ "condition": {
+ "funcName": "fluid.tests.fluid5282check",
+ "args": ["{that}", "${audio}"]
+ }
+ }
+ },
+ model: {
+ audio: "available"
+ },
+ renderOnInit: true
+ });
+
+ fluid.tests.fluid5282check = function (that, audioValue) {
+ that.lastAudioValue = audioValue;
+ };
+
+ jqUnit.test("FLUID-5282: protoComponent expansion should respect floating model reference", function () {
+ var that = fluid.tests.fluid5282root("#FLUID-5282");
+ jqUnit.assertEquals("Initial model value evaluated", "available", that.lastAudioValue);
+ that.applier.change("audio", "unavailable");
+ that.refreshView();
+ jqUnit.assertEquals("Updated model value evaluated", "unavailable", that.lastAudioValue);
+ });
+
+
jqUnit.module("Protocomponent Expander Tests");
jqUnit.test("makeProtoExpander Basic Tests", function () {
@@ -1814,6 +1963,7 @@ fluid.registerNamespace("fluid.tests");
},
renderOnInit: true
});
+
jqUnit.test("FLUID-4986: Select", function () {
var that = fluid.tests.fluid4986("#FLUID-4986");
jqUnit.assertEquals("Select should be rendered properly", that.model.select, that.locate("select").val());
@@ -1831,4 +1981,5 @@ fluid.registerNamespace("fluid.tests");
"${{test}.string}", that.locate("simpleBound6").text());
});
};
+
})(jQuery);
Something went wrong with that request. Please try again.