[PATCH v5 2/6] cgit.js: line range highlight: introduce javascript

[Andy Green]

On 06/28/2018 02:02 AM, Jason A. Donenfeld wrote:
> Hi Andy,
> 
> I'm super hesitant about the Pandora's box that introducing javascript
> implies, but perhaps there's no use in fighting the future.

The "fighting the future" bus left the station ten years ago when github 
was founded.  Worrying about JS is just fighting the past.

> A few notes:
> 
> - Your js needs the copyright line like the rest of the project.
> - Rather than awkwardly namespacing global methods, wrap everything
> inside a big anonymous function.

OK.

> - 1.5s is way too long for an animation. In fact, I'm not sure what an
> animation communicates at all, and perhaps we could get rid of it.

We can't apply the highlight until the page completes its load event, 
because the vertical layout is subject to be adjusted by image loads etc 
right up until then.  It looked nicer to me to have the highlight 
"become apparent" in the case the lines had already been visibly 
rendered without it.

> - Setting colors from inside js is a big no-no. Instead, add and
> remove classes from elements, and leave the styling to css.

OK.

> Also, can this be done from pure css? For example, certainly
> highlighting one line in pure css based on the #anchor is possible,
> via css link selectors.

Maybe you are thinking something I'm missing, but there are three 
unrelated <td>s there that happen to be rendered vertically aligned. 
They're not on a single <tr> despite it may look like that.

-Andy

> Jason
>