Closed Bug 1438072 Opened 2 years ago Closed 2 years ago

Change the looks for item of animation which does not change the value

Categories

(DevTools :: Inspector: Animations, enhancement)

enhancement
Not set

Tracking

(firefox61 fixed)

RESOLVED FIXED
Firefox 61
Tracking Status
firefox61 --- fixed

People

(Reporter: daisuke, Assigned: daisuke)

References

(Blocks 1 open bug)

Details

Attachments

(4 files)

We had done this behavior in original animation inspector.
In this bug, address following things.

* put the item at bottom of list
* change the color or opacity
Comment on attachment 8951149 [details]
Bug 1438072 - Part 0: Rename variables that are related to animated property item.

https://reviewboard.mozilla.org/r/220406/#review226918
Attachment #8951149 - Flags: review?(gl) → review+
Attachment #8951150 - Flags: review?(gl) → review?(pbrosset)
Attachment #8951151 - Flags: review?(gl) → review?(pbrosset)
Attachment #8951152 - Flags: review?(gl) → review?(pbrosset)
Comment on attachment 8951150 [details]
Bug 1438072 - Part 1: Put unchanged item to bottom of list.

https://reviewboard.mozilla.org/r/220408/#review232706
Attachment #8951150 - Flags: review?(pbrosset) → review+
Comment on attachment 8951151 [details]
Bug 1438072 - Part 2: Change the opacity of unchanged items.

https://reviewboard.mozilla.org/r/220410/#review232708

::: commit-message-6c2b7:1
(Diff revision 2)
> +Bug 1438072 - Part 2: Cnange the opacity of item. r?gl

nit: Change, not Cnange.
Also, maybe rephrase to:
Change the opacity of unchanged items.
Attachment #8951151 - Flags: review?(pbrosset) → review+
Comment on attachment 8951152 [details]
Bug 1438072 - Part 3: Add test.

https://reviewboard.mozilla.org/r/220412/#review232710

Looks good Daisuke, but could you please move this new part to a new test instead? 
Something like browser_animation_animated-unchanged-property-list.js.
This way, each test is simple to read, and fast to run, and only tets one thing only.
Attachment #8951152 - Flags: review?(pbrosset)
Thank you very much, Patrick!
I modified all your indicated points.
try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=babc582680577e11f2ab437f72c964249f833f2b
Comment on attachment 8951152 [details]
Bug 1438072 - Part 3: Add test.

https://reviewboard.mozilla.org/r/220412/#review232730

::: devtools/client/inspector/animation/test/browser_animation_animated-property-list_unchanged-items.js:7
(Diff revision 3)
> +   http://creativecommons.org/publicdomain/zero/1.0/ */
> +
> +"use strict";
> +
> +// Test following animated property list with unchanged items.
> +// * position and opacity of unchanged animated property item.

This needs rephrasing. This sentence doesn't mean much now that it's been extracted out of the other test. Please remove the asterisk too.
Attachment #8951152 - Flags: review?(pbrosset) → review+
Pushed by dakatsuka@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/45be7dba8b4c
Part 0: Rename variables that are related to animated property item. r=gl
https://hg.mozilla.org/integration/autoland/rev/b69717aaf597
Part 1: Put unchanged item to bottom of list. r=pbro
https://hg.mozilla.org/integration/autoland/rev/e14f469f1167
Part 2: Change the opacity of unchanged items. r=pbro
https://hg.mozilla.org/integration/autoland/rev/53316cd82eb4
Part 3: Add test. r=pbro
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.