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

Add and fix mouse events on timeline #2473

Merged
merged 12 commits into from
Dec 31, 2016
Merged

Conversation

yotamberk
Copy link
Contributor

@yotamberk yotamberk commented Dec 17, 2016

fixes #226, #2421 and #2429

// timeline.on('mouseDown', function (properties) {
// logEvent('mouseDown', properties);
// });

Copy link
Member

Choose a reason for hiding this comment

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

empty line

@@ -120,15 +120,27 @@ function Timeline (container, items, groups, options) {
this.itemsData = null; // DataSet
this.groupsData = null; // DataSet

this.on('tap', function (event) {
Copy link
Member

Choose a reason for hiding this comment

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

Did you make sure this still workes on mobile?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yup, I have a touch screen on my computer and it seems to work fine.

Copy link
Member

Choose a reason for hiding this comment

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

cool! But we should test it on mobile devices. It's possible that your screen emulates mouse events. I'll try to have a look on some devices.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Np

Copy link
Member

Choose a reason for hiding this comment

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

Ok, looks good on Android and iOS.

<td>
Passes a properties object as returned by the method <a href="#getEventProperties"><code>Timeline.getEventProperties(event)</code></a>.
</td>
<td>Fired when mouse over the Timeline.
Copy link
Member

Choose a reason for hiding this comment

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

Better: "Fired when the mouse hovers over a timeline element."

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's not really hover. I mean, it is in a sense, but from what I saw there is a difference between hover and mouseover events so I still want this to say "mouse over"

Copy link
Member

Choose a reason for hiding this comment

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

No problem. But the event is not fired by mouse over the "timeline" itself, but the event is fired if the mouse is over a "timeline element". Check it again, I'm not so sure myself but it looked like the event is e.g. fired by mouse over time-ticks, elements. Maybe we should clarify this!?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will do. I think ill change it to mouseover for the timeline as a whole

<td>
Passes a properties object as returned by the method <a href="#getEventProperties"><code>Timeline.getEventProperties(event)</code></a>.
</td>
<td>Fired when mouse down inside the Timeline.
Copy link
Member

Choose a reason for hiding this comment

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

I don't understand this event. I only managed to trigger this in the example by pressing the middle-mouse-button.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It fires when you press the left mouse button in the timeline

Copy link
Member

Choose a reason for hiding this comment

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

I get a "click" event on the left button and a "contextmenu" on the right button.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Weird... ill check that out

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've removed this event. I see absolutely no real reason for this event.

</tr>

<tr>
<td>mouseUp</td>
Copy link
Member

Choose a reason for hiding this comment

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

I don't understand this event. I only managed to trigger this in the example by pressing the middle-mouse-button.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same as above just for when you release it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've removed this event. I see absolutely no real reason for this event.

this.dom.root.onmouseup = function (event) {
me.emit('mouseUp', me.getEventProperties(event))
};
this.dom.root.onmousemove = function (event) {
Copy link
Member

Choose a reason for hiding this comment

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

do we really want to emit this event. This happens quite often? What is the usecase for this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see a use for it when someone wants a tooltip appearing any time you're on the timeline and want to see the exact time you are hovering on

@@ -120,15 +120,27 @@ function Timeline (container, items, groups, options) {
this.itemsData = null; // DataSet
this.groupsData = null; // DataSet

this.on('tap', function (event) {
Copy link
Member

Choose a reason for hiding this comment

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

Ok, looks good on Android and iOS.

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.

None yet

2 participants