Merge pull request #116 from offirgolan/fixes-on-fixes

Code cleanup and logic fixes

Author: offirgolan

addon/-private/Record.js

@@ -1,13 +1,18 @@
+import Ember from 'ember';
 import { unwrapValue } from 'ember-time-machine/utils/value';
 
+const {
+  isArray
+} = Ember;
+
 export default class Record {
-  constructor(target, path, key, before, after, isArray = false) {
+  constructor({ target, path, key, before, after }) {
     this.target = target;
     this.path = path;
     this.key = key;
     this.before = unwrapValue(before);
     this.after = unwrapValue(after);
-    this.isArray = isArray;
+    this.isArray = isArray(target) && typeof key === 'number';
 
     this.pathToKey = path.join('.');
     this.fullPath = path.concat(this.key).join('.');

addon/mixins/time-machine.js

@@ -230,7 +230,7 @@ export default Ember.Mixin.create({
     let content = this.get('content');
 
     if (isArray(content)) {
-      return emberArray(content).invoke(...arguments);
+      return emberArray(content).invoke(methodName, ...args);
     } else {
       return tryInvoke(content, methodName, args);
     }
@@ -323,7 +323,9 @@ export default Ember.Mixin.create({
         } else {
           RecordUtils.redoArrayRecord(record);
         }
-      } else if (isLast || record.fullPath !== nextRecord.fullPath) {
+      } else if (isLast ||
+          record.fullPath !== nextRecord.fullPath ||
+          record.target !== nextRecord.target) {
         /*
           Apply the last object property change that occured in a row.
           ex) If firstName changed 5 times in a row and we undo, then apply only
@@ -340,10 +342,10 @@ export default Ember.Mixin.create({
    * Extract the specified number of records from the given stack
    *
    * @method _extractRecords
-   * @param  {String}      type       'undo' or 'redo'
-   * @param  {Number}      numRecords Number of records to apply
-   * @param  {Object}      options
-   * @return {Array}                  Records that were extracted
+   * @param  {Array} stack
+   * @param  {Number} numRecords Number of records to apply
+   * @param  {Object} options
+   * @return {Array} Records that were extracted
    * @private
    */
   _extractRecords(stack, numRecords, options = {}) {

addon/proxies/array.js

@@ -12,6 +12,7 @@ export default Ember.ArrayProxy.extend(RecordKeeperMixin, {
   replaceContent(startIndex, numRemoved, objects) {
     let state = this.get('_rootMachineState');
     let path = this.get('_path');
+    let content = this.get('content');
     let before, after;
 
     if (state && !pathInGlobs(path.join('.'), state.get('frozenProperties'))) {
@@ -21,7 +22,13 @@ export default Ember.ArrayProxy.extend(RecordKeeperMixin, {
         after = objects;
       }
 
-      this._addRecord(new Record(this.get('content'), path, startIndex, before, after, true));
+      this._addRecord(new Record({
+        target: content,
+        path,
+        key: startIndex,
+        before,
+        after
+      }));
 
       return this._super(startIndex, numRemoved, unwrapValue(objects));
     }

addon/proxies/object.js

@@ -15,7 +15,14 @@ export default Ember.ObjectProxy.extend(RecordKeeperMixin, {
     let path = this.get('_path');
 
     if (state && !pathInGlobs(path.concat(key).join('.'), state.get('frozenProperties'))) {
-      this._addRecord(new Record(content, path, key, content.get(key), value, false));
+      this._addRecord(new Record({
+        target: content,
+        path,
+        key,
+        before: content.get(key),
+        after: value
+      }));
+
       return this._super(key, unwrapValue(value));
     }
 

addon/utils/utils.js

@@ -1,17 +1,15 @@
-import require from 'require';
-
-export function requireModule(module) {
-  return require.has(module) ? require(module)['default'] : undefined;
-}
-
 export function pathInGlobs(path = '', globs = []) {
   for (let i = 0; i < globs.length; i++) {
-    let regex = (`.${globs[i]}`).replace(new RegExp(/\.\*/, 'g'), '\\.(\\w+)').replace(new RegExp(/\.@each/, 'g'), '\\.(\\d+)');
-
-    if ((`.${path}`).match(regex)) {
+    if ((`.${path}`).match(regexFromGlob(globs[i]))) {
       return true;
     }
   }
 
   return false;
 }
+
+function regexFromGlob(glob) {
+  return (`.${glob}`)
+          .replace(new RegExp(/\.\*/, 'g'), '\\.(\\w+)')
+          .replace(new RegExp(/\.@each/, 'g'), '\\.(\\d+)');
+}

addon/utils/value.js

@@ -17,7 +17,7 @@ export function wrapValue(obj, key, value) {
     return availableMachines.get(value);
   }
 
-  if (shouldWrapValue(...arguments)) {
+  if (shouldWrapValue(obj, key, value)) {
     if (isArray(value)) {
       Machine = TimeMachine.Array;
     } else if (typeof value === 'object') {
@@ -40,12 +40,12 @@ export function wrapValue(obj, key, value) {
 }
 
 export function unwrapValue(value) {
-  if (value && isArray(value)) {
-    return value.map((v) => get(v, 'isTimeMachine') ? unwrapValue(get(v, 'content')) : v);
+  if (value && get(value, 'isTimeMachine')) {
+    return unwrapValue(get(value, 'content'));
   }
 
-  if (value && (value instanceof Ember.ObjectProxy || get(value, 'isTimeMachine'))) {
-    return unwrapValue(get(value, 'content'));
+  if (value && isArray(value)) {
+    return value.map((v) => get(v, 'isTimeMachine') ? unwrapValue(get(v, 'content')) : v);
   }
 
   return value;
@@ -54,12 +54,11 @@ export function unwrapValue(value) {
 function shouldWrapValue(obj, key, value) {
   let state = obj.get('_rootMachineState');
   let maxDepth = state.get('maxDepth');
-  let shouldWrapValue = state.get('shouldWrapValue');
-
   let fullPath = obj.get('_path').concat(key);
   let valueType = typeOf(value);
-  let trackCurrDepth = maxDepth < 0 || fullPath.length <= maxDepth;
-  let correctValueType = valueType === 'object' || valueType === 'instance' || valueType === 'array';
 
-  return value && correctValueType && trackCurrDepth && !get(value, 'isTimeMachine') && shouldWrapValue(value, obj, key);
+  return (valueType === 'object' || valueType === 'instance' || valueType === 'array') &&
+         (maxDepth < 0 || fullPath.length <= maxDepth) &&
+         !get(value, 'isTimeMachine') &&
+         state.shouldWrapValue(value, obj, key);
 }

tests/dummy/app/controllers/index.js

@@ -1,5 +1,9 @@
 import Ember from 'ember';
 
+const {
+  isEmpty
+} = Ember;
+
 export default Ember.Controller.extend({
   store: Ember.inject.service(),
 
@@ -8,15 +12,20 @@ export default Ember.Controller.extend({
 
   actions: {
     addTask() {
-      let newTask = this.get('store').createRecord('task', {
-        title: this.get('newTask')
-      });
+      let title = this.get('newTask');
+
+      if (isEmpty(title)) {
+        return;
+      }
+
+      let newTask = this.get('store').createRecord('task', { title });
 
       if (this.get('model.settings.newOnTop')) {
         this.get('model.tasks').insertAt(0, newTask);
       } else {
         this.get('model.tasks').pushObject(newTask);
       }
+
       this.set('newTask', '');
     },
     removeTask(task) {

tests/dummy/app/models/user.js

@@ -7,9 +7,12 @@ const { computed } = Ember;
 export default DS.Model.extend({
   firstName: attr('string'),
   lastName: attr('string'),
-  username: attr('string'),
   avatar: attr('string'),
 
+  username: computed('firstName', function() {
+    return `${this.get('firstName')}ster`;
+  }).readOnly(),
+
   settings: DS.belongsTo('setting'),
   tasks: DS.hasMany('task'),
 

tests/dummy/app/styles/app.scss

@@ -105,6 +105,7 @@ nav {
       margin: 0;
     }
     .new-task-icon {
+      cursor: pointer;
       margin-top: 30px;
       font-size: 36px;
       font-weight: 200;

tests/dummy/app/templates/index.hbs

@@ -3,15 +3,11 @@
 	<div class="row">
 		<div class="col s4">
 			{{#md-card title=model.displayName image=model.avatar activator=true class="user-card"}}
-				{{#md-card-content}}
-					{{!-- <div class="row center-align">
-						<img src={{model.avatar}} alt="" width="200px" class="circle deep-orange lighten-2 responsive-img"> <!-- notice the "circle" class -->
-					</div> --}}
-		    {{/md-card-content}}
+				{{#md-card-content}}{{/md-card-content}}
 
 				{{#md-card-reveal}}
 					<div class="row">
-						{{md-input value=model.username label='Username' class="col s12"}}
+						{{md-input value=model.username label='Username' class="col s12" disabled=true}}
 						{{md-input value=model.firstName label='First Name' class="col s12"}}
 						{{md-input value=model.lastName label='Last Name' class="col s12"}}
 					</div>
@@ -29,7 +25,7 @@
 							<br>
 							<p>{{md-switch checked=model.settings.showTabs name="Show Tabs"}}</p>
 							<br>
-							<p>{{md-switch checked=model.settings.sortableCards name="Use Sortable Cards"}}</p>
+							<p>{{md-switch checked=model.settings.sortableCards name="Sortable Cards"}}</p>
 						</div>
 					</div>
 		    {{/md-card-content}}
@@ -41,7 +37,7 @@
 					{{#md-card-content}}
 						<div class="row">
 							<div class="col s1 center-align">
-								<span class="new-task-icon">+</span>
+								<span class="new-task-icon" {{action 'addTask'}}>+</span>
 							</div>
 							<div class="col s11">
 									{{input value=newTask placeholder="What needs to be done?" enter=(action 'addTask')}}
@@ -109,25 +105,25 @@
 	class="fixed-btns"
 	btnIcon="fa fa-undo"
 	btnClass="btn-large deep-orange darken-1"
-	action="undo"}}
+	action=(action 'undo')}}
 
 	{{md-fixed-btn
 		btnClass="blue-grey lighten-1"
 		btnIcon="fa fa-floppy-o"
-		action="commit"}}
+		action=(action 'commit')}}
 
 	{{md-fixed-btn
 		btnClass="yellow darken-1"
 		btnIcon="fa fa-repeat all"
-		action="redoAll"}}
+		action=(action 'redoAll')}}
 
 	{{md-fixed-btn
 		btnClass="amber lighten-1"
 		btnIcon="fa fa-undo all"
-		action="undoAll"}}
+		action=(action 'undoAll')}}
 
 	{{md-fixed-btn
 		btnClass="orange darken-1"
 		btnIcon="fa fa-repeat"
-		action="redo"}}
+		action=(action 'redo')}}
 {{/md-fixed-btns}}