Fix click and doubleclick events on items #2988
Conversation
There was a problem hiding this comment.
This is the best review I can do without testing the changes - I don't see how I can do that now.
Also, extra: in the timeline basicUsage example, I get this.groupsData === null in the first test of ItemSet.prototype.groupFromTarget(). Is that supposed to happen?
lib/timeline/component/item/Item.js
Outdated
| @@ -145,16 +145,26 @@ Item.prototype.repositionY = function() { | |||
| */ | |||
| Item.prototype._repaintDragCenter = function () { | |||
| if (this.selected && this.options.editable.updateTime && !this.dom.dragCenter) { | |||
| var me = this; | |||
|
|
|||
| var me = this | |||
lib/timeline/component/item/Item.js
Outdated
| // create and show drag area | ||
| var dragCenter = document.createElement('div'); | ||
| dragCenter.className = 'vis-drag-center'; | ||
| dragCenter.dragCenterItem = this; | ||
| this.hammer = new Hammer(dragCenter) |
There was a problem hiding this comment.
So hammer is now a member var; but it will be overwritten on every redraw. Is this what is supposed to happen? Doesn't look right to me.
I would expect it to be either a local var, or a more permanent member var.
lib/timeline/component/item/Item.js
Outdated
| item: me.id | ||
| }); | ||
| }); | ||
| this.hammer.on('doubletap', function (event) { |
There was a problem hiding this comment.
Do tap and doubletap always fire consecutively? Or is it either one or the other?
In the latter case, please consider usage of requireFailure().
There was a problem hiding this comment.
They don't fire consecutively. It's one or the other. There is no need for requireFailure() here because it hasn't been used anywhere else. I can add it in another PR.
lib/timeline/component/item/Item.js
Outdated
| event.stopPropagation(); | ||
| me.parent.itemSet._onUpdateItem(me); | ||
| me.parent.itemSet.body.emitter.emit('doubleClick', { |
There was a problem hiding this comment.
I am trusting that this block is correct; it is congruent to click above, so it looks OK.
not sure when this was introduced. |
lib/timeline/component/item/Item.js
Outdated
| // create and show drag area | ||
| var dragCenter = document.createElement('div'); | ||
| dragCenter.className = 'vis-drag-center'; | ||
| dragCenter.dragCenterItem = this; | ||
| this.hammer = new Hammer(dragCenter) | ||
| var hammer = new Hammer(dragCenter) |
There was a major bug I encountered trying to catch click and doubleclick events on selected items.
This PR solves this bug. (continues #2473 to fully solve: #2421, )
checkout the vis/examples/timeline/interaction/eventListeners.html example in this PR to see the click\doubleclick event getting fired when an item is already selected.