1363346 - Use SVG context-fill instead of clip-path for animation-fast-track.svg

Status: RESOLVED FIXED

(DevTools :: Inspector: Animations, enhancement, P3)

Description

[Tim Nguyen :ntim]

in https://dxr.mozilla.org/mozilla-central/source/devtools/client/themes/animationinspector.css#459 :
-moz-context-properties: fill;
fill: (...)

then in the SVG:

<!-- This Source Code Form is subject to the terms of the Mozilla Public
   - License, v. 2.0. If a copy of the MPL was not distributed with this
   - file, You can obtain one at http://mozilla.org/MPL/2.0/. -->
<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 9 12" width="16" height="16">
  <path d="M5.75 0l-1 5.5 2 .5-3.5 6 1-5-2-.5z" fill="context-fill"/>
</svg>

Comment 1

[[:Towkir] Ahmed]

Attachment: svg-context-fill.patch

Looks good on my local build, hope it works ;)

Comment 2

[Tim Nguyen :ntim]

Comment on attachment 8880275 [details] [diff] [review]
svg-context-fill.patch

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

::: devtools/client/themes/animationinspector.css
@@ +475,3 @@
>  }
>  
>  .animation-timeline .some-properties .name::after {

I know it wasn't part of the bug initially, but the rules on .animation-timeline .some-properties .name::after and .animation-timeline .all-properties .name::after are identical.

Can you remove this block, and just share the selector ?

.animation-timeline .all-properties .name::after,
.animation-timeline .some-properties .name::after {
  -moz-context-properties: fill;
  ...
}

Comment 3

[Tim Nguyen :ntim]

Thanks for working on this btw, the patch works great!

Comment 4

[Tim Nguyen :ntim]

Comment on attachment 8880275 [details] [diff] [review]
svg-context-fill.patch

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

::: devtools/client/themes/animationinspector.css
@@ +469,5 @@
>  
>  .animation-timeline .all-properties .name::after {
> +  -moz-context-properties: fill;
> +  fill: var(--theme-content-color3);
> +  background-image: url(images/animation-fast-track.svg#thunderbolt);

Oh, I almost forgot, can you remove #thunderbolt from the URL ? #thunderbolt references to the old clip path, that's now removed.

Comment 5

[[:Towkir] Ahmed]

Should I nit the code like this ?:
> background-image: url("images/animation-fast-track.svg");

I am talking about the quotation, currently there is no quotes.

Comment 6

[[:Towkir] Ahmed]

Attachment: svg-context-fill.patch

However, I have made the changes. Please have a look.

Comment 7

[Tim Nguyen :ntim]

(In reply to [:Towkir] Ahmed from comment #5)
> Should I nit the code like this ?:
> > background-image: url("images/animation-fast-track.svg");
> 
> I am talking about the quotation, currently there is no quotes.

I don't mind either way to be honest.

Comment 8

[Tim Nguyen :ntim]

Comment on attachment 8882161 [details] [diff] [review]
svg-context-fill.patch

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

Looks good to me!

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/aab62d15c1ca
Use SVG context-fill instead of clip-path for animation-fast-track.svg now. r=ntim

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/aab62d15c1ca