Closed Bug 1156757 Opened 6 years ago Closed 6 years ago

Decommission the animation-inspector UI v2

Categories

(DevTools :: Inspector: Animations, defect)

defect
Not set
normal

Tracking

(firefox43 fixed, relnote-firefox 43+)

RESOLVED FIXED
Firefox 43
Tracking Status
firefox43 --- fixed
relnote-firefox --- 43+

People

(Reporter: pbro, Assigned: pbro)

References

Details

(Keywords: dev-doc-complete)

Attachments

(1 file)

Once the new UI is ready (bug 1153271), the pref introduced in bug 1156754 can be removed, the old UI be removed (and corresponding tests), and the v3 UI switched on by default.
Blocks: 1153271
This is the big move from v2 to v3.
This patch removes all the v2 code that's no longer needed:
- everything that has to do with the former PlayerWidgets (since we now use a timeline component),
- the auto-refresh mechanism of the animationplayer fronts,
- all tests that are no longer valid with v3
- and this also removes the pref that v3 was developed behind
Assignee: nobody → pbrosset
Status: NEW → ASSIGNED
Attachment #8662906 - Flags: review?(past)
Tests pass locally, but here's also a try build: https://treeherder.mozilla.org/#/jobs?repo=try&revision=975fbd4c9191
Comment on attachment 8662906 [details] [diff] [review]
Bug_1156757_-_Turn_ON_the_animation_inspector_UI_v.diff

Review of attachment 8662906 [details] [diff] [review]:
-----------------------------------------------------------------

Looks fine to me, assuming the UI can still connect to an old server.
Attachment #8662906 - Flags: review?(past) → review+
Thanks Panos.
To be honest, I haven't tested connecting to an old server in a while, but the new UI doesn't have specific server-requirements. It's always been using the same server features as the older UI.
Also, every server features I've been adding during the course of this year was always added with a corresponding client-side fallback.
https://hg.mozilla.org/mozilla-central/rev/d26831f27e14
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 43
Release Note Request (optional, but appreciated)
[Why is this notable]: New animation inspector UI enabled
[Suggested wording]: Animation inspector now displays animations in a timeline
[Links (documentation, blog post, etc)]:
relnote-firefox: --- → ?
(In reply to Tim Nguyen [:ntim] from comment #7)
> [Links (documentation, blog post, etc)]:
https://medium.com/@patrickbrosset/inspecting-animations-in-firefox-f1f5fd941567

This will be part of the marketing campaign in November.
Oh, I completely forgot to flag this with dev-doc-needed. 
On MDN, this part will need to be updated: https://developer.mozilla.org/en-US/docs/Tools/Page_Inspector/How_to/Work_with_animations#Animation_inspector
Keywords: dev-doc-needed
Patrick, I've written some stuff up here: https://developer.mozilla.org/en-US/docs/Tools/Page_Inspector/How_to/Work_with_animations#Firefox_43.

I've tried doing something a bit different with this. Rather than just one video or a bunch of text and screenshots, I've tried to mix up very short videos, text, screenshots, and a live sample, in a way that's hopefully more engaging. I'd like to know if you think it works. Also whether I've missed anything important.
Flags: needinfo?(pbrosset)
This looks really good, thanks.
One comment however is that the page mentions "CSS property transition" in multiple places whereas the panel can display both CSS transitions *and* CSS animations. I think the first sentence for example:
"the animation inspector displays property transitions synchronized along a timeline"
should be changed to something like this:
"the animation inspector displays css animations and css transitions synchronized along a timeline".

In the case where the panel shows a css animation, it won't show the property for example, because there may be several of them, changed mutliple times across the course of the animation using @keyframes rules. So it will show the name of the animation instead.

I'll do some minor text changes on MDN to reflect this.

Thanks Will for your work on this.
Flags: needinfo?(pbrosset)
For info Will, I've marked my changes on the MDN article as needing an editorial review, to make sure I didn't do any silly wording/grammar mistakes.
Noted for 43, with the suggested link.
Component: Developer Tools: Inspector → Developer Tools: Animation Inspector
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.