Update Column.js to support parent #348
Conversation
|
@alexander-alvarez what is the reason behind this PR? |
|
if you're doing data manipulation / formatting it's nice to have a reference to the parent column while you're in the subColumn -- I'm adding these at instantiation, so it's nbd, but seemed logical that if we have the relationship one way, we'd have it the other way -- like ember-data inverses |
addon/classes/Column.js
Outdated
| @@ -333,7 +342,7 @@ export default class Column extends Ember.Object.extend({ | |||
|
|
|||
| let { subColumns } = options; | |||
|
|
|||
| subColumns = emberArray(makeArray(subColumns).map((sc) => new Column(sc))); | |||
| subColumns = emberArray(makeArray(subColumns).map((sc) => merge(sc, { parent: this })).map((sc) => new Column(sc))); | |||
| subColumns.setEach('_group', this); | |||
There was a problem hiding this comment.
@alexander-alvarez the parent is already being set here. Maybe we can just make this public API?
There was a problem hiding this comment.
lol...
the. next. line.
and I didn't even see it
There was a problem hiding this comment.
I would vote for making it public.
what would we want to call it:
parent?
parentColumn?
group?
There was a problem hiding this comment.
LOL its all good. I think parent works okay. Make sure you change the rest of _group to parent. I think it's referenced in a couple places.
addon/mixins/draggable-column.js
Outdated
| }).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'); |
There was a problem hiding this comment.
@alexander-alvarez 'dragColumnGroup' should be 'parent' for both here.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Right but the dragColumnGroup exists on the column component not the column instance. You are currently always comparing undefined to undefined.
There was a problem hiding this comment.
yup... I'm going to show myself the door... so off these days man...
There was a problem hiding this comment.
LOL its all good! I got you 😄
dragColumnGroup does not exist on the column class.
|
Looks good to me! Thanks for seeing this through! I'll make a patch release a little later today 😸 |
|
@alexander-alvarez v1.8.4 released. |
No description provided.