Add Nested groups #2416
Add Nested groups #2416
Conversation
…into nested-groups
| @@ -25,231 +25,232 @@ var BACKGROUND = '__background__'; // reserved group id for background items wit | |||
| * @extends Component | |||
| */ | |||
| function ItemSet(body, options) { | |||
| this.body = body; | |||
There was a problem hiding this comment.
You updated the indent to tabs. I like that! Sadly this makes this Pull-Request un-reviewable. Could you maybe change that back?
There was a problem hiding this comment.
Can you recommend an easy way to revert this specific commit?
lib/timeline/component/Group.js
Outdated
| } | ||
| this.dom.label = label; | ||
|
|
||
| var collapseGroupBtn = document.createElement('div'); |
There was a problem hiding this comment.
I think the whole collapseGroupBtncan be removed, now that the arrow is done is CSS.
| } | ||
|
|
||
| .vis-nested-group { | ||
| background: #e5e5e5; |
There was a problem hiding this comment.
I personally would prefer something even a little lighter like #f5f5f5
lib/timeline/component/Group.js
Outdated
| if (this.showNested) { | ||
| util.removeClassName(this.dom.label, 'collapsed'); | ||
| util.addClassName(this.dom.label, 'expanded'); | ||
| // this.dom.collapseGroupBtn.innerHTML = '▼'; |
There was a problem hiding this comment.
This comments can be removed to keep the code clean
|
@mojoaxel Fixed the comments =) |
mojoaxel
left a comment
mojoaxel
left a comment
There was a problem hiding this comment.
still a few wrong indents, but otherwise very nice!
lib/timeline/component/ItemSet.js
Outdated
| groupsData.update(targetGroup); | ||
| this.options.groupOrderSwap(draggedGroup, targetGroup, groupsData); | ||
| groupsData.update(draggedGroup); | ||
| groupsData.update(targetGroup); |
lib/timeline/component/ItemSet.js
Outdated
| me.options.groupOrderSwap(switchGroup, shouldBeGroup, dataset); | ||
| groupsData.update(switchGroup); | ||
| groupsData.update(shouldBeGroup); | ||
| groupsData.update(shouldBeGroup); |
mojoaxel
left a comment
mojoaxel
left a comment
There was a problem hiding this comment.
I get an error Cannot redraw item: no parent attached running this new example. I'm not sure if it's a problem introduced by this PR but we should have a look.
| { | ||
| id: 13, | ||
| content: "clean house", | ||
|
|
| <td>Provides a means to toggle the whether a group is displayed or not. Defaults to <code>true</code>.</td> | ||
| </tr> | ||
| <tr> | ||
| <td>nestedGroups</td> |
There was a problem hiding this comment.
Now that I look at it I think "subGroups" would have been the better work, but I'm ok with this.
There was a problem hiding this comment.
subGroups refer to groups in a group and not nested underneath. So I actually prefer nestedGroups.
|
|
||
| // create a dataset with items | ||
| var items = new vis.DataSet(); | ||
| var groupIds = groups.getIds() |
| id: 3, | ||
| content: "John", | ||
| nestedGroups: [14], | ||
| showNested: false |
There was a problem hiding this comment.
The showNested: false causes the bad error Cannot redraw item: no parent attached. Looks like Items can not be attached to groups that are not visible.
This needs to be fixed !
There was a problem hiding this comment.
I'm on it at this moment. It's a weird problem. trying to figure out what is causing this exactly...
|
@mojoaxel Hey! I just managed to figure out how to fix that warning that occurred with my PR. Took me 4 days but managed eventually... It seems like it was a warning that already occurred with the addition of the visible group. Trying to initialize a group with |
|
@mojoaxel Can you please review this again and accept it? |
2 participants
solves #2300