Decommission the animation-inspector UI v2

RESOLVED FIXED in Firefox 43

Status

defect
RESOLVED FIXED
4 years ago
Last year

People

(Reporter: pbro, Assigned: pbro)

Tracking

({dev-doc-complete})

unspecified
Firefox 43
Dependency tree / graph

Firefox Tracking Flags

(firefox43 fixed, relnote-firefox 43+)

Details

Attachments

(1 attachment)

Assignee

Description

4 years ago
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.
Assignee

Updated

4 years ago
Blocks: 1153271
Assignee

Comment 1

4 years ago
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)
Assignee

Comment 2

4 years ago
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+
Assignee

Comment 4

4 years ago
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: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 43

Comment 7

4 years ago
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: --- → ?
Assignee

Comment 8

4 years ago
(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.
Assignee

Comment 9

4 years ago
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)
Assignee

Comment 11

4 years ago
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)
Assignee

Comment 12

4 years ago
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.
Assignee

Updated

4 years ago
Component: Developer Tools: Inspector → Developer Tools: Animation Inspector

Updated

Last year
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.