Skip to content

Commit

Permalink
Drop uniqueid binding and move "id" logic to wrapper: major wrapper r…
Browse files Browse the repository at this point in the history 
…efactoring (Fix #641)

Main change outside from wrapper is content._unwrap() doesn't exists anymore and is replaced by content._plainObject() that also accept writes: content._plainObject(newContentPlainObject).
Added a couple of test cases for the wrapper.
  • Loading branch information
@bago
bago committed May 4, 2022
1 parent 2d92fda commit f26de10
Show file tree
Hide file tree
Showing 10 changed files with 186 additions and 194 deletions.
2 changes: 1 addition & 1 deletion spec/converter-spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ describe('Template converter', function() {
}, {
optionalName: 'simpleBlock',
templateMode: 'show',
html: '<div data-bind="attr: { id: id }, uniqueId: $data"><div data-bind="wysiwygId: id()+\'_text\', wysiwygClick: function(obj, evt) { $root.selectItem(text, $data); return false }, clickBubble: false, wysiwygCss: { selecteditem: $root.isSelectedItem(text) }, scrollIntoView: $root.isSelectedItem(text), wysiwygOrHtml: text"></div></div>'
html: '<div data-bind="attr: { id: id }"><div data-bind="wysiwygId: id()+\'_text\', wysiwygClick: function(obj, evt) { $root.selectItem(text, $data); return false }, clickBubble: false, wysiwygCss: { selecteditem: $root.isSelectedItem(text) }, scrollIntoView: $root.isSelectedItem(text), wysiwygOrHtml: text"></div></div>'
}];

expect(parseData.templates).toEqual(expectedTemplates);
Expand Down
80 changes: 64 additions & 16 deletions spec/wrapper-spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,33 @@ describe('model wrapper and undomanager', function() {
undoserializer = require("../src/js/undomanager/undoserializer.js");
undoManager = require('../src/js/undomanager/undomanager.js');
modelDef = require('../src/js/converter/model.js');

});

beforeEach(function() {
global.document = {
lookedup: {},
getElementById: function(id) {
if (this.lookedup.hasOwnProperty(id)) return true;
this.lookedup[id] = 1;
return false;
}
}
});

afterEach(function() {
delete global.document;
});

it('should not alter input object on push', function() {
var templateDef = JSON.parse("" + fs.readFileSync("spec/data/template-versafix-1.def.json"));
var content = main.wrappedResultModel(templateDef);
var titleBlock = modelDef.generateModel(templateDef._defs, 'titleBlock');
var originalDef = ko.toJSON(titleBlock);
var blocks = content().mainBlocks().blocks;
blocks.push(titleBlock);
var newDef = ko.toJSON(titleBlock);
expect(originalDef).toEqual(newDef);
});

it('should be able to load previous data and deal with variants', function() {
Expand All @@ -26,7 +53,7 @@ describe('model wrapper and undomanager', function() {
var content = main.wrappedResultModel(templateDef);

var savedModel = JSON.parse("" + fs.readFileSync("spec/data/template-versafix-1.save1.json"));
content._wrap(savedModel);
content._plainObject(savedModel);

// loaded correctly?
expect(content().mainBlocks().blocks()[2]().titleText()).toEqual("My title");
Expand All @@ -38,13 +65,12 @@ describe('model wrapper and undomanager', function() {
});

it('should support undo/redo and full model replacement', function() {

var templateDef = JSON.parse("" + fs.readFileSync("spec/data/template-versafix-1.def.json"));
var content = main.wrappedResultModel(templateDef);

var titleBlock = modelDef.generateModel(templateDef._defs, 'titleBlock');

var jsonContent = ko.toJSON(content);
var jsonContent = ko.toJSON(content._plainObject());

var undoRedoStack = undoManager(content, {
levels: 100,
Expand Down Expand Up @@ -73,7 +99,7 @@ describe('model wrapper and undomanager', function() {
content().mainBlocks().blocks.push(titleBlock);

var unwrapped = ko.utils.parseJson(jsonContent);
content._wrap(unwrapped);
content._plainObject(unwrapped);

expect(content().titleText()).toEqual("TITLE");

Expand All @@ -91,17 +117,14 @@ describe('model wrapper and undomanager', function() {
expect(content().titleText()).toEqual("New Title 2");

var unwrapped = ko.utils.parseJson(jsonContent);
content._wrap(unwrapped);
content._plainObject(unwrapped);

expect(content().titleText()).toEqual("TITLE");

// Fails on current code because the "undo" of a full content _wrap doesn't wrap the previous model
/*
// Fails on current code because the "undo" of a full content _plainObject doesn't wrap the previous model
undoRedoStack.undoCommand.execute();

expect(content().titleText()).toEqual("New Title 2");
*/

});

it('should support undo/redo of move actions', function() {
Expand All @@ -113,7 +136,7 @@ describe('model wrapper and undomanager', function() {
var titleBlock2 = modelDef.generateModel(templateDef._defs, 'titleBlock');
var titleBlock3 = modelDef.generateModel(templateDef._defs, 'titleBlock');

var jsonContent = ko.toJSON(content);
var jsonContent = ko.toJSON(content._plainObject());

var undoRedoStack = undoManager(content, {
levels: 100,
Expand Down Expand Up @@ -201,12 +224,10 @@ describe('model wrapper and undomanager', function() {
expect(blocks()[1]().text()).toEqual("Title 2");
expect(blocks()[2]().text()).toEqual("Title 3");

// using "move action" (sortable with move strategy will use valueWillMute/hasMutated
blocks.valueWillMutate();
var underlyingBlocks = ko.utils.unwrapObservable(blocks);
var removed2 = underlyingBlocks.splice(0, 1);
underlyingBlocks.splice(2, 0, removed2[0]);
blocks.valueHasMutated();
// using separate remove-add actions without valueWillMutate and without manual declaration of the mergemode.
// maybe this case is no more needed with the updated knockout/knockout-sortable libraries that correctly detect the move
var removed2 = blocks.splice(0, 1);
blocks.splice(2, 0, removed2[0]);

debug("F", blocks);

Expand All @@ -232,6 +253,33 @@ describe('model wrapper and undomanager', function() {

});

it('should not merge removal of a block being just added', function() {
var templateDef = JSON.parse("" + fs.readFileSync("spec/data/template-versafix-1.def.json"));
var content = main.wrappedResultModel(templateDef);
var titleBlock = modelDef.generateModel(templateDef._defs, 'titleBlock');
var jsonContent = ko.toJSON(content._plainObject());

var undoRedoStack = undoManager(content, {
levels: 100,
undoLabel: ko.computed(function() { return "Undo (#COUNT#)"; }),
redoLabel: ko.computed(function() { return "Redo"; })
});
undoRedoStack.setUndoActionMaker(undoserializer.makeUndoAction.bind(undefined, content));
undoserializer.watchEnabled(true);
undoRedoStack.setModeOnce();

var blocks = content().mainBlocks().blocks;

blocks.push(titleBlock);
expect(blocks().length).toEqual(1);

blocks.remove(blocks()[0]);
expect(blocks().length).toEqual(0);

undoRedoStack.undoCommand.execute();
expect(blocks().length).toEqual(1);
});

afterAll(function() {
mockery.disable();
mockery.deregisterAll();
Expand Down
26 changes: 0 additions & 26 deletions src/js/bindings/virtuals.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,32 +4,6 @@
var ko = require("knockout");
var console = require("console");

ko.bindingHandlers['uniqueId'] = {
currentIndex: 0,
'init': function(element, valueAccessor) {
var data = ko.utils.unwrapObservable(valueAccessor()) || {};
if (data.id() === '') {
var id, el, prefix;
// TODO we need a better prefix
prefix = 'ko_' + (typeof data.type !== 'undefined' ? ko.utils.unwrapObservable(data.type) : 'block');
// when loading an exising model, IDs could be already assigned.
do {
id = prefix + '_' + (++ko.bindingHandlers['uniqueId'].currentIndex);
el = global.document.getElementById(id);
if (el) {
// when loading an existing model my "currentIndex" is empty.
// but we have existing blocks, so I must be sure I don't reuse their IDs.
// We use different prefixes (per block type) so that a hidden block
// (for which we have no id in the page, e.g: preheader in versafix-1)
// will break everthing once we reuse its name.
}
} while (el);
data.id(id);
}
}
};
ko.virtualElements.allowedBindings['uniqueId'] = true;

ko.bindingHandlers['virtualAttr'] = {
update: function(element, valueAccessor) {
if (element.nodeType !== 8) {
Expand Down
3 changes: 1 addition & 2 deletions src/js/converter/main.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,11 @@ var modelDef = require("./model.js");
var wrappedResultModel = function(templateDef) {
var defs = templateDef._defs;
var templateName = templateDef.templateName;
var finalModelContentDef = modelDef.getDef(defs, templateName);

var finalModelContent = modelDef.generateResultModel(templateDef);

var wrapper = require("./wrapper.js");
var res = wrapper(finalModelContent, finalModelContentDef, defs);
var res = wrapper(defs, finalModelContent);

return res;
};
Expand Down
3 changes: 2 additions & 1 deletion src/js/converter/parser.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@ var processStyle = function(element, templateUrlConverter, bindingProvider, addU
var newStyle = null;
var newBindings;
if (addUniqueId) newBindings = {
uniqueId: '$data',
// unique id is assigned automatically by the wrapper (to prevent 2 separate async operations)
// uniqueId: '$data',
attr: {
id: 'id'
}
Expand Down
Loading

0 comments on commit f26de10

Please sign in to comment.