Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update Column.js to support parent #348

Merged
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 12 additions & 11 deletions addon/classes/Column.js
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,17 @@ export default class Column extends Ember.Object.extend({
*/
minResizeWidth: 0,

/**
* The parent column (or group) for this sub-column.
* This will only have a value if this column is a sub-column.
* Note: this doesn't update if you move this sub-column to another parent after instantiation.
*
* @property parent
* @type Column
* @optional
*/
parent: null,

/**
* An array of sub columns to be grouped together
* @property subColumns
Expand Down Expand Up @@ -273,16 +284,6 @@ export default class Column extends Ember.Object.extend({
return guidFor(this);
}).readOnly(),

/**
* A reference to the belonging column group. This will only have
* a value if this column is a sub column.
*
* @property format
* @type {Function}
* @private
*/
_group: null,

/**
* True if `hidden` or `responsiveHidden` is true.
* @property isHidden
Expand Down Expand Up @@ -334,7 +335,7 @@ export default class Column extends Ember.Object.extend({
let { subColumns } = options;

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

this.set('subColumns', subColumns);
}
Expand Down
17 changes: 13 additions & 4 deletions addon/mixins/draggable-column.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,17 +25,26 @@ export default Ember.Mixin.create({
}
}).readOnly(),

dragColumnGroup: computed('column._group', function() {
let columnGroup = this.get('column._group');
return columnGroup ? columnGroup.get('subColumns') : this.get('table.columns');
/**
* Array of Columns indicating where the column can be potentially dragged.
* If the column is part of a group (has a parent column), this will be all of the columns in that group,
* otherwise it's all of the columns in the table.
*
* @property dragColumnGroup
* @type Array
* @readonly
*/
dragColumnGroup: computed('column.parent', function() {
let parent = this.get('column.parent');
return parent ? parent.get('subColumns') : this.get('table.columns');
}).readOnly(),

isDropTarget: computed(function() {
let column = this.get('column');
/*
A column is a valid drop target only if its in the same group
*/
return column.get('droppable') && column.get('_group') === sourceColumn.get('_group');
return column.get('droppable') && column.get('dragColumnGroup') === sourceColumn.get('dragColumnGroup');
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@alexander-alvarez 'dragColumnGroup' should be 'parent' for both here.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's the same thing.
I was going off of the comment: A column is a valid drop target only if its in the same group, so decided to use the variable that had the word group in it, instead of parent.

dragColumnGroup is derived from parent, so if parent's are equal, then the dragColumnGroup will be equal, and if no parents are defined:
null === null will be truthy just as
this.get('table.columns') === this.get('table.columns') is.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right but the dragColumnGroup exists on the column component not the column instance. You are currently always comparing undefined to undefined.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yup... I'm going to show myself the door... so off these days man...

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LOL its all good! I got you 😄

}).volatile().readOnly(),

dragStart(e) {
Expand Down
11 changes: 11 additions & 0 deletions tests/unit/classes/column-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -72,3 +72,14 @@ test('CP - visibleSubColumns', function(assert) {
col.set('hidden', true);
assert.equal(col.get('visibleSubColumns.length'), 0);
});

test('subColumns / parent', function(assert) {
let col = new Column({
subColumns: [{}]
});
assert.ok(col);
assert.equal(col.subColumns.length, 1);

assert.equal(col.subColumns[0].get('parent'), col);

});