663100 - [highlighter] Re-implement the highlighting mechanism in HTML + flex box model

Status: RESOLVED FIXED

(DevTools :: General, defect)

Description

[Paul Rouget [:paul]]

The current implementation (bug 642471) is in Canvas. Using HTML will allow us to use CSS transitions for animations, and will make the design work easier.

Comment 1

[Paul Rouget [:paul]]

Attachment: patch v1

Comment 2

[Mihai Sucan [:msucan]]

Comment on attachment 538467 [details] [diff] [review]
patch v1

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

Really good work. I have a few nit picks, but r+!

Your highlighter.css changes also need to be made in pinstripe and winstripe.

I am taking this patch into the main highlighter patch. It's surgical in its changes, very clean and straight-forward. Thank you!

(I applied all the nits my self, so I was able to test the patch. So no need for an update.)

::: browser/base/content/highlighter.xhtml
@@ +48,5 @@
> +    divs, organized in 3 rows, keeping the div in the middle transparent.
> +-->
> +<div id="veil-container">
> +<div id="veil">
> +    <div id="veil-topbox" class="veil"></div>

The other empty divs are marked as <div /> but this one is <div></div>.

@@ +56,5 @@
> +        <div id="veil-rightbox" class="veil"/>
> +    </div>
> +    <div id="veil-bottombox" class="veil"/>
> +</div>
> +</div>

Indentation is not consistent, and it should be two spaces.

::: browser/themes/gnomestripe/browser/highlighter.css
@@ +16,5 @@
>    z-index: 1;
>  }
>  
> +.veil {
> +    background-color: rgba(0, 0, 0, 0.5);

Indentation needs to follow the same level as in the rest of the file: two spaces.

@@ +21,3 @@
>  }
> +.veil, #veil-middlebox, #veil-transparentbox {
> +    -moz-transition: 0.2s;

I'd say 0.2s is too slow. I am going for 0.1s.

@@ +25,4 @@
>  }
> +#veil-container {
> +    position: absolute;
> +    top: 0; left: 0;

Each property needs to be on its own line.