The graduations displayed in the timeline header of the animation-inspector could be better.
As discussed. Let me know if you have questions.
I made a few changes to the codebase in order to address this problem.
I did not launch the tests nor edited them as I wanted to know if I'm heading into the right direction.
The main idea is to compute an optimal number of intervals rather than directly an interval length, in order to ensure that it shows the max time ( and that the ticks are even, or sort of ).
I did some minor change to the CSS to get some space and display the max time ( the last tick ) in its full length.
I also changed the function that draw the background ticks but as I'm not very familiar with canvas, and even less with bitwise operators, I don't know if I'm doing a good job here.
Thanks Nicolas for working on this.
The intervals look better than today for sure.
I think we could do something even better though.
- on jsbin.com (there's an animation when you click on the logo, top left), making the timeline quite big (~700px), I got: 0, 24, 48, 72, 96, etc. It's not too bad, but it would make even more sense to users if this was 0, 25, 50, 75, 100, etc. Easier to relate to. I know that the animation on that page lasts for 240ms, so cutting it in 25ms intervals would mean that the last interval be smaller. And I think that's what we can improve with your solution: accepting the last interval to be cut off.
In short, I think we should stick to showing intervals multiple of 1, 2.5, 5 only. Because these are more logical amounts of time, they're easier to reason about when looking at the timeline.
So, for instance, if you had a total length of 283ms, then (depending on how much width there is), you'd cut it in 0, 50, 100, 150, 200, 250, 300 (which means that the last interval would be somewhat smaller than the other, partly hidden in fact).
Not sure how I feel about this. On one hand it's nice to be able to see the last interval's label on the right, on the other hand that means we're using more horizontal space now, and because the animation inspector is in a sidebar panel for now, it's usually going to be very narrow. So 40px to the right just to display a time label sounds like a waster of space.
Especially because the left-hand panel (that contains the list of DOM nodes) is already pretty wide.
If you try and make the sidebar width be ~300px on, say, http://tympanus.net/Development/DialogEffects/ricky.html (which I assume would be a usual width for users), then the animations are barely visible.
I don't think loosing this information (the last time interval) is a big deal either, knowing that the current time is always displayed in the toolbar.
I edited my patch with your suggestions ( the jsbin was very helpful ), and I think I got something quite good (at least, it looks good with all the examples you gave in Comment 3).
There are obviously some tests that needs to be rewritten ( devtools/client/animationinspector/test/unit/test_findOptimalTimeInterval.js and devtools/client/animationinspector/test/browser_animation_timeline_header.js ) but I wanted to make sure that you are happy with those changes before doing it.
> I don't think loosing this information (the last time interval) is a big deal either, knowing that the current time is always displayed in the toolbar.
I put a title attribute on the time intervals, so when it is cut, the user can see the whole text hovering the label. What do you think about this ?
This looks great. Thanks Nicolas!
nit: no need for parenthesis around the values of the last 2 variables.
I like the idea, but I'm not sure we should do it this way. Indeed, tick labels have a certain width, so you could in theory, have your mouse positioned at 205ms from the start, in the timeline, but since the label is right below your mouse, then you'd see, say, the "200ms" label.
I think if we wanted to do something like this, we should have a mouseover listener to shows a label continuously when you move your mouse over the timeline graduations, with a small vertical line that indicates exactly your position.
So I'd suggest removing this for now.
I removed the title attribute and edited the two tests that needed it because of the changes in the findOptimalTimeInterval function.
I ran the whole test suite and everything is fine.
I hope it looks good to you.
I'm glad it is good.
Oh, I was looking forward to do it as I now have access to TRY :) . Maybe on the next bug thus.
> I'm glad it is good.
> the next bug thus.
Try push went really fast today! And it's green, so I'll land this now. Thanks a lot Nicolas!