Skip to content
This repository has been archived by the owner on Jul 29, 2019. It is now read-only.

Adding 3rd level hierarchy for nested groups fixing #2846 #3940

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from
Open

Adding 3rd level hierarchy for nested groups fixing #2846 #3940

wants to merge 2 commits into from

Conversation

Jogai
Copy link

@Jogai Jogai commented Apr 20, 2018

fixes #2846

Taken changes from @JobrianTrinidad

@Jogai Jogai changed the title Merged Jobrian's changes && tried formatting to project style Adding 3rd level hierarchy for nested groups fixing #2846 Apr 20, 2018
@avrahamcool
Copy link

any news on this?

Copy link
Contributor

@yotamberk yotamberk left a comment

Choose a reason for hiding this comment

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

Hi @Jogai ,
I appreciate the PR, but I cannot accept it like this. You've changed the nestedInGroup property and introduced a breaking change without changing the docs.
You've also applied a lot of spacing changes which make it very difficult to understand the actual changes you've added.
Please add this change to the docs and remove all the spacing so I can add this.

I've also now branched out to a new fork of my own including only Timeline.
If you can reapply the change here: https://github.com/yotamberk/timeline-plus
you will find this fix there within a week or two.

Awesome fix! Thanks for your contribution! Future PRs will be reviewed and added in the new fork

@Jogai
Copy link
Author

Jogai commented Jul 2, 2018

Thank you for looking into this. I'll see what I can do

@jgorene
Copy link

jgorene commented Jul 7, 2018

Hi,
Well, I tested "timeline-plus" by @yotamberk to see what it could do...
Good work for this very interesting new feature, thanks to all!
A priori, it works well except to collapse all levels of a group directly from "first level" but maybe I missed something!
This action will be possible after the application of this fix?
Thank you again ;)

@yotamberk
Copy link
Contributor

This has been applied properly in timeline-plus v2.1.8
I've now branched out to a new fork of my own including only Timeline.
I've applied your change here: https://github.com/yotamberk/timeline-plus
and can be used via npm here: https://www.npmjs.com/package/timeline-plus
Awesome fix! Thanks for your contribution! Future PRs will be reviewed and added in the new fork

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants