The default bug view has changed. See this FAQ.

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

RESOLVED FIXED

Status

()

Firefox
Developer Tools
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: paul, Assigned: paul)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

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

Comment 1

6 years ago
Created attachment 538467 [details] [diff] [review]
patch v1
Attachment #538467 - Flags: review?(mihai.sucan)
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.
Attachment #538467 - Flags: review?(mihai.sucan) → review+
(Assignee)

Updated

6 years ago
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.