-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Conversation
docs/timeline/index.html
Outdated
@@ -944,6 +944,14 @@ <h2 id="Configuration_Options">Configuration Options</h2> | |||
</tr> | |||
|
|||
<tr> | |||
<td>onInitialDrawComplete</td> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Small niggle on my part: The options are arranged alphabetically, this new entry and onUpdate
are not.
@@ -34,7 +34,8 @@ | |||
|
|||
var container = document.getElementById('visualization'); | |||
var options = { | |||
editable: true | |||
editable: true, | |||
onInitialDrawComplete: function() { return logEvent('Timeline initial draw completed', {}) }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return
not necessary. logEvent
returns nothing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
;
@@ -1003,7 +1003,6 @@ Core.prototype._redraw = function() { | |||
} else { | |||
this.redrawCount = 0; | |||
} | |||
this.initialDrawDone = true; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesn't this need to be set? It's the only place in the module where it goes to true
.
OK, got it. It's set in Timeline._activateOnInitialDrawDone()
. I find it a bit confusing that this is in a derived class.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Timeline._activateOnInitialDrawDone()
was incorrect. I removed it.
lib/timeline/Timeline.js
Outdated
if (!this.initialDrawDone) { | ||
this.initialDrawDone = true; | ||
setTimeout(() => { | ||
if (this.options.onInitialDrawComplete) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why bother with setTimeOut
if onInitialDrawComplete
is not present? Better to move this condition out of setTimeOut
.
lib/timeline/Timeline.js
Outdated
} | ||
else { | ||
me.fit({animation: false}); | ||
me.setWindow(start, end, {animation: false}, this._activateOnInitialDrawDone.bind(this)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Strictly speaking, calling it here is a lie; _redraw()
is done at the end of this method. initialDrawDone
should be set to true after that call, same applies to the callback.
lib/timeline/Timeline.js
Outdated
@@ -411,21 +421,22 @@ Timeline.prototype.focus = function(id, options) { | |||
* provided to specify duration and easing function. | |||
* Default duration is 500 ms, and default easing | |||
* function is 'easeInOutQuad'. | |||
* @param {Function} callback |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@param {function} [callback]
- casing, callback optional
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK looks good. Just that one unrelated question about the double code tags.
docs/timeline/index.html
Outdated
<tr> | ||
<td>onMoving</td> | ||
<td>function</td> | ||
<td>none</td> | ||
<td>Callback function triggered repeatedly when an item is being moved. See section <a href="#Editing_Items">Editing Items</a> for more information. Only applicable when both options <code>selectable</code> and <code>editable.updateTime</code> or <code>editable.updateGroup</code> are set <code><code>true</code></code>. | ||
</td> | ||
</tr> | ||
|
||
<tr> | ||
<td>onRemove</td> | ||
<td>function</td> | ||
<td>none</td> | ||
<td>Callback function triggered when an item is about to be removed: when the user tapped the delete button on the top right of a selected item. See section <a href="#Editing_Items">Editing Items</a> for more information. Only applicable when both options <code>selectable</code> and <code>editable.remove</code> are set <code><code>true</code></code>. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Er, why the double <code>
tags? It happens more often in the docs.
@@ -111,6 +111,11 @@ Core.prototype._create = function (container) { | |||
this._redraw(); | |||
} | |||
}.bind(this)); | |||
this.on('rangechanged', function () { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, this is a better place.
lib/timeline/Timeline.js
Outdated
} | ||
} | ||
|
||
if (!me.initialDrawDone && me.initialRangeChangeDone) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah yes right. If I was just a bit clearer in my head I should have remarked on that. Make sense, not directly connected to the while-loop.
@wimrijnders seems like that |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK! We're done here. Let's get this thing merged.
* initial trial * Add onInitialDrawComplete * Add docs * Add to eventListeners examples * Keeping things DRY * Remove callback insertion * Remove call * Fix initial real first draw complete and fix comments from review * remove all <code><code>
Solves #3529