Closed Bug 1211810 Opened 5 years ago Closed 5 years ago
Lock highlighter on animated elements in the animation-inspector
The animated elements that are shown in the animation-inspector panel allow 2 mouse interactions: - hovering over them highlights them in the page (the highlighter stays visible only as long as the mouse is over the element in the panel), - clicking on the inspector icon next to them selects them in the markup-view We'd like to add a third type of interaction: locking the highlighter on an element. The idea is that it might be useful to see the highlighter on an element while you're using the timeline. For this to work, we'd need a way to show the highlighter, and then hide it.
Assignee: nobody → pbrosset
Status: NEW → ASSIGNED
This patch modifies the way DOM nodes are previewed in the animation-inspector. Now, clicking on the DOM node output itself will select it in the inspector (and there's a tooltip for this), and clicking on the inspector icon *next* to the DOM node output will "lock" the highlighter on the node, in the page. This proves to be very useful when you want to track an element that's being transformed in an animation. You can use the tool to highlight the element, and then scrub back and forth to see it being transformed easily. This uses a new instance of the highlighter so that it doesn't interfere with the other highlighter that is used on node hover in the markup-view, breadcrumbs, etc. This patch also: - removes some old unused code in animation-panel.js that I had forgotten to remove before, - adds a new test - modifies a couple of existing tests so they pass. Pending try build: https://treeherder.mozilla.org/#/jobs?repo=try&revision=1075094fc413
Comment on attachment 8671398 [details] [diff] [review] Bug_1211810_-_Add_a_way_to_lock_the_highlighter_on.diff Review of attachment 8671398 [details] [diff] [review]: ----------------------------------------------------------------- Looks good to me. ::: devtools/client/animationinspector/utils.js @@ +146,5 @@ > + * that is used to lock the highlighter on animated nodes in the page. > + * It instantiates a new highlighter that is then shared amongst all instances > + * of AnimationTargetNode. This is useful because that means showing the > + * highlighter on one animated node will unhighlight the previously highlighted > + * one, but will not interfer with the default inspector highlighter. Typo, "interfere".
Attachment #8671398 - Flags: review?(ttromey) → review+
Why the change from .node-selector to .node-highlighter? This impacts themes unnecessarily.
(In reply to Alfred Kayser from comment #6) > Why the change from .node-selector to .node-highlighter? > This impacts themes unnecessarily. That's right, I guess I did not think of this. Which themes do you have in mind? To my knowledge, there's only the light/dark themes which we maintain ourselves and the firebug theme. To answer your question, I changed the classname because node-highlighter is more meaningful in this case than node-selector.
See: https://addons.mozilla.org/en-US/firefox/complete-themes/ I know that Mozilla is actively pushing these away, but there are still many people using and developing these themes.
I've added a section on Firefox 44: https://developer.mozilla.org/en-US/docs/Tools/Page_Inspector/How_to/Work_with_animations#Firefox_44 Also see bug 1205681, comment 15 for more on why I've chosen to present it like this.
LGTM, thanks Will!
Component: Developer Tools: Inspector → Developer Tools: Animation Inspector
You need to log in before you can comment on or make changes to this bug.