Skip to content

Commit

Permalink
[BREAKING] fixes #444 #662 converts ES6 native classes to ember objects
Browse files Browse the repository at this point in the history
This is a breaking change as it converts the new Class() syntax to Class.create().
In addition, the only way I could get these to work was to convert the positional arguments to named args in an options hashes:

`new Table(columns, rows)` => `Table.create(columns: columns, rows: rows)`
`new Row(content, options)` => `Row.create({ content: })`
`new Column(opts)` => `Column.create(opts)`

We’ve had to replace the native constructor methods with emberObject init methods as per the official docs which should also fix #699. As a result we’ve removed the Ember 2.12 ‘hacks’ which shouldn’t be an issue as we’ve bumped ember version minimum version to 2.18.
  • Loading branch information
fran-worley committed Aug 19, 2019
1 parent ef66756 commit bf2fcde
Show file tree
Hide file tree
Showing 18 changed files with 151 additions and 201 deletions.
26 changes: 4 additions & 22 deletions addon/classes/Column.js
Original file line number Diff line number Diff line change
Expand Up @@ -315,33 +315,15 @@ export default class Column extends EmberObject.extend({
return emberArray(isHidden ? [] : subColumns.filterBy('isHidden', false));
}).readOnly(),

init(...args) {
this._super(...args);
init(options = {}) {
this.setProperties(options);

const subColumns = emberArray(makeArray(this.get('subColumns')).map((sc) => new Column(sc)));
const subColumns = emberArray(makeArray(this.get('subColumns')).map((sc) => Column.create(sc)));
subColumns.setEach('parent', this);

this.set('subColumns', subColumns);
}
}) {
/**
* @class Column
* @constructor
* @param {Object} options
*/
constructor(options = {}) {
// TODO: Revert this, when babel#5862 is resolved.
// https://github.com/babel/babel/issues/5862
// HACK: Passing properties to super instead of manually setting them fixes the
// implicit run loop creation for Ember 2.12.
// https://travis-ci.org/offirgolan/ember-light-table/jobs/344818839#L790
super(options);

if (options instanceof Column) {
return options;
}
}
}
}) {}

// https://github.com/offirgolan/ember-light-table/issues/436#issuecomment-310138868
fixProto(Column);
21 changes: 1 addition & 20 deletions addon/classes/Row.js
Original file line number Diff line number Diff line change
Expand Up @@ -77,26 +77,7 @@ export default class Row extends ObjectProxy.extend({
rowId: computed(function() {
return guidFor(this);
}).readOnly()
}) {
/**
* @class Row
* @constructor
* @param {Object} content
* @param {Object} options
*/
constructor(content, options = {}) {
// TODO: Revert this, when babel#5862 is resolved.
// https://github.com/babel/babel/issues/5862
// HACK: Passing properties to super instead of manually setting them fixes the
// implicit run loop creation for Ember 2.12.
// https://travis-ci.org/offirgolan/ember-light-table/jobs/344818839#L790
super(Object.assign({}, options, { content }));

if (content instanceof Row) {
return content;
}
}
}
}) {}

// https://github.com/offirgolan/ember-light-table/issues/436#issuecomment-310138868
fixProto(Row);
43 changes: 25 additions & 18 deletions addon/classes/Table.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import Column from 'ember-light-table/classes/Column';
import SyncArrayProxy from 'ember-light-table/-private/sync-array-proxy';
import { mergeOptionsWithGlobals } from 'ember-light-table/-private/global-options';
import fixProto from 'ember-light-table/utils/fix-proto';
import { assign } from '@ember/polyfills';
import { isNone } from '@ember/utils';

const RowSyncArrayProxy = SyncArrayProxy.extend({
Expand Down Expand Up @@ -133,43 +134,41 @@ export default class Table extends EmberObject.extend({
arr.pushObjects(c.get('isGroupColumn') ? c.get('subColumns') : [c]);
return arr;
}, emberArray([]));
}).readOnly()
}) {
}).readOnly(),

/**
* @class Table
* @constructor
* @param {Array} columns
* @param {Array} rows
* @param {Object} options
* @param {Array} options.columns
* @param {Array} options.rows
* @param {Boolean} options.enableSync If `true`, creates a two way sync
* between the table's rows and the passed rows collection. Also see
* `setRowsSynced(rows)`.
* @param {Object} options.rowOptions Options hash passed through to
* `createRow(content, options)`.
*/
constructor(columns = [], rows = [], options = {}) {
super();

init(options = {}) {
let { columns = [], rows = [] } = options;
assert('[ember-light-table] columns must be an array if defined', isArray(columns));
assert('[ember-light-table] rows must be an array if defined', isArray(rows));

let _options = mergeOptionsWithGlobals(options);
let _columns = emberArray(Table.createColumns(columns));
let _rows = emberArray(Table.createRows(rows, _options.rowOptions));
this.setProperties(mergeOptionsWithGlobals(options));

this.set('columns', emberArray(Table.createColumns(columns)));

let _rows = emberArray(Table.createRows(rows, this.get('rowOptions')));

if (_options.enableSync) {
if (this.get('enableSync')) {
_rows = RowSyncArrayProxy.create({
syncArray: rows,
content: _rows
});
}

this.setProperties({
columns: _columns,
rows: _rows
});
this.set('rows', _rows);
}

}) {
destroy() {
this._super(...arguments);

Expand Down Expand Up @@ -411,7 +410,11 @@ export default class Table extends EmberObject.extend({
* @return {Row}
*/
static createRow(content, options = {}) {
return new Row(content, options);
if (content instanceof Row) {
return content;
} else {
return Row.create(assign({}, options, { content }));
}
}

/**
Expand All @@ -434,7 +437,11 @@ export default class Table extends EmberObject.extend({
* @return {Column}
*/
static createColumn(column) {
return new Column(column);
if (column instanceof Column) {
return column;
} else {
return Column.create(column);
}
}

/**
Expand Down
6 changes: 3 additions & 3 deletions addon/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import Row from './classes/Row';
* ```javascript
* import Table from 'ember-light-table';
*
* const table = new Table(columns, rows, options);
* const table = Table.create({ columns: columns, rows: rows });
* ```
*
* Here is a more real-word example
Expand Down Expand Up @@ -59,7 +59,7 @@ import Row from './classes/Row';
* }),
*
* table: computed('model', function() {
* return new Table(this.get('columns'), this.get('model'));
* return Table.create({ columns: this.get('columns'), rows: this.get('model') });
* })
* });
* ```
Expand All @@ -72,7 +72,7 @@ import Row from './classes/Row';
* ```javascript
* import Table from 'ember-light-table';
*
* const table = new Table(columns, model, { enableSync: true });
* const table = Table.create({ columns: columns, rows: model, enableSync: true });
* ```
*
* The `enableSync` options creates a __two way__ sync. This means that any manipulation
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ test('it renders', function(assert) {
});

test('it renders value', function(assert) {
this.set('column', new Column({ valuePath: 'foo' }));
this.set('row', new Row({ foo: 'bar' }));
this.set('column', Column.create({ valuePath: 'foo' }));
this.set('row', Row.create({ content: { foo: 'bar' } }));

this.render(hbs`{{light-table/cells/<%= dasherizedModuleName %> column row rawValue=(get row column.valuePath)}}`);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ test('it renders', function(assert) {
});

test('it renders label', function(assert) {
this.set('column', new Column({ label: '<%= dasherizedModuleName %>' }));
this.set('column', Column.create({ label: '<%= dasherizedModuleName %>' }));

this.render(hbs`{{light-table/columns/<%= dasherizedModuleName %> column}}`);

Expand Down
2 changes: 1 addition & 1 deletion tests/dummy/app/mixins/table-common.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ export default Mixin.create({
init() {
this._super(...arguments);

let table = new Table(this.get('columns'), this.get('model'), { enableSync: this.get('enableSync') });
let table = Table.create({ columns: this.get('columns'), rows: this.get('model'), enableSync: this.get('enableSync') });
let sortColumn = table.get('allColumns').findBy('valuePath', this.get('sort'));

// Setup initial sort column
Expand Down
18 changes: 9 additions & 9 deletions tests/integration/components/light-table-occlusion-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ module('Integration | Component | light table | occlusion', function(hooks) {
});

test('it renders', async function(assert) {
this.set('table', new Table());
this.set('table', Table.create());
await render(hbs `{{light-table table height="40vh" occlusion=true estimatedRowHeight=30}}`);

assert.equal(find('*').textContent.trim(), '');
Expand All @@ -32,7 +32,7 @@ module('Integration | Component | light table | occlusion', function(hooks) {
test('scrolled to bottom', async function(assert) {
assert.expect(4);

this.set('table', new Table(Columns, this.server.createList('user', 50)));
this.set('table', Table.create({ columns: Columns, rows: this.server.createList('user', 50) }));

this.set('onScrolledToBottom', () => {
assert.ok(true);
Expand All @@ -58,7 +58,7 @@ module('Integration | Component | light table | occlusion', function(hooks) {

test('fixed header', async function(assert) {
assert.expect(2);
this.set('table', new Table(Columns, this.server.createList('user', 5)));
this.set('table', Table.create({ columns: Columns, rows: this.server.createList('user', 5) }));

await render(hbs `
{{#light-table table height='500px' id='lightTable' occlusion=true estimatedRowHeight=30 as |t|}}
Expand All @@ -81,7 +81,7 @@ module('Integration | Component | light table | occlusion', function(hooks) {

test('fixed footer', async function(assert) {
assert.expect(2);
this.set('table', new Table(Columns, this.server.createList('user', 5)));
this.set('table', Table.create({ columns: Columns, rows: this.server.createList('user', 5) }));

await render(hbs `
{{#light-table table height='500px' id='lightTable' occlusion=true estimatedRowHeight=30 as |t|}}
Expand All @@ -104,7 +104,7 @@ module('Integration | Component | light table | occlusion', function(hooks) {

test('table assumes height of container', async function(assert) {

this.set('table', new Table(Columns, this.server.createList('user', 5)));
this.set('table', Table.create({ columns: Columns, rows: this.server.createList('user', 5) }));
this.set('fixed', true);

await render(hbs `
Expand All @@ -121,7 +121,7 @@ module('Integration | Component | light table | occlusion', function(hooks) {
});

test('table body should consume all available space when not enough content to fill it', async function(assert) {
this.set('table', new Table(Columns, this.server.createList('user', 1)));
this.set('table', Table.create({ columns: Columns, rows: this.server.createList('user', 1) }));
this.set('fixed', true);

await render(hbs `
Expand All @@ -148,7 +148,7 @@ module('Integration | Component | light table | occlusion', function(hooks) {

this.owner.register('component:custom-row', RowComponent);

this.set('table', new Table(Columns, this.server.createList('user', 1)));
this.set('table', Table.create({ columns: Columns, rows: this.server.createList('user', 1) }));

await render(hbs `
{{#light-table table occlusion=true estimatedRowHeight=30 as |t|}}
Expand All @@ -170,7 +170,7 @@ module('Integration | Component | light table | occlusion', function(hooks) {
}));

let users = this.server.createList('user', 3);
this.set('table', new Table(Columns, users));
this.set('table', Table.create({ columns: Columns, rows: users }));

await render(hbs `
{{#light-table table height='500px' occlusion=true estimatedRowHeight=30 as |t|}}
Expand Down Expand Up @@ -222,7 +222,7 @@ module('Integration | Component | light table | occlusion', function(hooks) {
cellComponent: 'some-component'
}];

this.set('table', new Table(columns, [{}]));
this.set('table', Table.create({ columns, rows: [{}] }));

this.actions.someAction = () => {
assert.ok(true, 'table action is passed');
Expand Down
Loading

0 comments on commit bf2fcde

Please sign in to comment.