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

Clustering support POC #87

Merged
merged 12 commits into from
Jan 2, 2019
Merged

Clustering support POC #87

merged 12 commits into from
Jan 2, 2019

Conversation

konsvarl
Copy link
Contributor

Clustering support POC, you could find example here: examples\other\clustering.html

@konsvarl konsvarl changed the title Clustering Clustering support POC Dec 27, 2018
@konsvarl konsvarl changed the base branch from master to develop December 28, 2018 11:20
@vlsi
Copy link

vlsi commented Dec 28, 2018

@konsvarl , would you please try

const end = start.clone().add(getRandomArbitrary(1, 60*24*4), 'm');

?

Days number: 4, orders per day number: 758

It makes clustering.html unusable.

@konsvarl
Copy link
Contributor Author

konsvarl commented Dec 28, 2018

@vlsi , are you sure? It work fine form me with "Enable clustering" checkbox checked.

I could observe some performance degradation when zooming-in clusters till individual ranges become visible, but that lags isn't critical, but I can work on optimization.

@vlsi
Copy link

vlsi commented Dec 28, 2018

http://recordit.co/0NyLuQlldD

As you see, the items are jumping, and it is somewhat hard to identify them (as you scroll, the item might jump to completely different place)

@konsvarl
Copy link
Contributor Author

konsvarl commented Dec 28, 2018

This is because of current stacking implementation: each time items go out of visible range, the rest items jump. I committed a small fix, please try to scroll items with that fix.

The same jumping behavior could be observed even without clustering on a huge data set.
To fix this problem completely we need to completely redesign stacking workflow taking into account items outside view-port, but this is another big task.

PS. I realize that this is a dirty hack, but user still able to select the item to identify

Copy link
Owner

@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.

Wow... Really amazing work!
Can you please add these options to the docs?

@vlsi
Copy link

vlsi commented Dec 28, 2018

@konsvarl , it looks like the jumping happens much less frequent now.

@konsvarl
Copy link
Contributor Author

konsvarl commented Dec 29, 2018

@yotamberk, I had updated the doc, but I noticed that none of the examples could be opened from the site.

@yotamberk
Copy link
Owner

Awesome! Thank you again for this contribution!

@yotamberk yotamberk merged commit 0dc0d79 into yotamberk:develop Jan 2, 2019
@Blinnikov
Copy link

Hi guys, when is it going to be released as a new version?

@yotamberk
Copy link
Owner

@Blinnikov it's up now

@yotamberk
Copy link
Owner

Hey @konsvarl ,
I know it's been a while since you've touched this PR but I ran in to a bug and tracing it back to where it starts, it seems that it all comes back to a small change done in this PR.
https://github.com/yotamberk/timeline-plus/pull/87/files#diff-f4d829b4f8b61a3d664fc7a5eb70924dR648
Can you explain why you added the batch argument?
It seems to work okay without this addition. With this addition, it seems that initial stacking for subgroups do not work.

@konsvarl
Copy link
Contributor Author

konsvarl commented Nov 1, 2019

Hey @konsvarl ,
I know it's been a while since you've touched this PR but I ran in to a bug and tracing it back to where it starts, it seems that it all comes back to a small change done in this PR.
https://github.com/yotamberk/timeline-plus/pull/87/files#diff-f4d829b4f8b61a3d664fc7a5eb70924dR648
Can you explain why you added the batch argument?
It seems to work okay without this addition. With this addition, it seems that initial stacking for subgroups do not work.

Hi, @yotamberk , sorry for such a late response. I got the case when many updates with huge amount of events come one after another, and I don't want to trigger visible items recalculation on each update. I think this logic could be still useful but with some extra-work: for instance we could inject flag to DataSet.Update or introduce something like DataSet.BeginUpdate()/FinishUpdate().
I could make PR with such additions. What do you think?

@yotamberk
Copy link
Owner

Sounds interesting. I had to disable the batch parameter because it would cause problems with not recalculating item stacking in subgroups. Maybe with the idea you've supplied, this might do the trick with not recalculating the huge amount of items and still prevent the bug with subgroup recalculations not occurring.
The work you've done is tremendous and highly appreciated. Any more contributions you supply would be highly valued and appreciated!

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

Successfully merging this pull request may close these issues.

4 participants