[PATCH v2] blame: css: make blame highlight div absolute and top left

[Andy Green]

On 06/20/2018 03:59 AM, John Keeping wrote:
> On Tue, Jun 19, 2018 at 03:11:52AM +0800, Andy Green wrote:
>> On June 19, 2018 2:57:47 AM GMT+08:00, John Keeping <john at keeping.me.uk> wrote:
>>> On Mon, Jun 18, 2018 at 02:02:54PM +0800, Andy Green wrote:
>>>> Normal operation of blame view requires div.highlight to
>>>> have absolute position and set to its parent's top left
>>>> for me.
>>>>
>>>> Otherwise the grey background boxes indicating the extent of
>>>> the patch in the lines td displace the highlit sources, they
>>>> start at the bottom of the td.
>>>>
>>>> This patch makes the blame highlight div start back up the top of
>>>> its parent area and render on top of the grey boxes.
>>>>
>>>> Checked on Linux Firefox 60 and Linux Chrome 69.
>>>
>>> Which browser is this broken in?  I tried Linux Firefox 60 and
>>> Chromium 67 and it looks ok without this patch.  (I'm not opposed to
>>> the patch in principle, indeed it seems like a sensible change, but
>>> I'm curious why I can't reproduce the problem.)
>>
>> It's not caused in the browser, I checked the cgit site blame and that
>> doesn't have the problem.  I also went back to check just master
>> without anything on top, and it's still broken on my box.  I confirmed
>> I'm using the latest css, and that snipping the div with the boxes in
>> the lines td (using developer tools / inspector in ffox) is also
>> enough to have it display correctly.
>>
>> Md2html seems to issue inline css style related to "highlight" as part
>> of its processing, I think as it is, that may put the actual rendering
>> behaviour at the mercy of pygments version or whatever actually
>> generates that.  So separating the css to a different name is likely
>> needed for robustness anyway.
> 
> Sounds sensible.  It looks like we use the same get_style_defs() call in
> both syntax-hightlighing.py and md2html.  In syntax-hightlighing.py
> the class name is generated by pygments [1] but in md2html we actually
> specify the class as a parameter to the codehilite extension.
> 
> Your patch series allows these filters to run in the same page for the
> first time, so I think what we should do is rename the highlight class
> in md2html.  Does that also fix the problem?

Yes, it also resolves the naming conflict and it can display correctly.

I'll switch to this method then.

-Andy

> [1] It may be possible to override it, I haven't checked the API docs
> 
>> My box is fedora 28.
>>
>> python3-pygments-2.2.0-10.fc28.noarch
>>
>>>> "highlight" div class name is also used in md2html rendering
>>>> output.  So this patch solves it by introducing a wrapper
>>>> div and new "blame_highlight" css class.
>>>>
>>>> Signed-off-by: Andy Green <andy at warmcat.com>
>>>> ---
>>>>   cgit.css   |    2 ++
>>>>   ui-blame.c |    4 ++--
>>>>   2 files changed, 4 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/cgit.css b/cgit.css
>>>> index da8d9b0..5a85ceb 100644
>>>> --- a/cgit.css
>>>> +++ b/cgit.css
>>>> @@ -162,6 +162,8 @@ div#cgit table.list
>>> tr.nohover-highlight:hover:nth-child(odd) {
>>>>   	background: white;
>>>>   }
>>>>   
>>>> +div#cgit div.blame_highlight { position: absolute; top: 0; left: 0;
>>> }
>>>
>>> Is the "left" needed here?  I don't see any problem with setting the
>>> position and top attributes, but setting left:0 moves the content right
>>> up against the cell boundary where before we had a slight gap.
>>
>> OK... it was not offset horizontally anyway so this is overkill.  I'll remove it.
>>
>> -Andy
>>
>>>> +
>>>>   div#cgit table.list th {
>>>>   	font-weight: bold;
>>>>   	/* color: #888;
>>>> diff --git a/ui-blame.c b/ui-blame.c
>>>> index 6e23f0b..ab44e3f 100644
>>>> --- a/ui-blame.c
>>>> +++ b/ui-blame.c
>>>> @@ -196,7 +196,7 @@ static void print_object(const struct object_id
>>> *oid, const char *path,
>>>>   	free((void *)sb.final_buf);
>>>>   
>>>>   	/* Lines */
>>>> -	html("<pre><code>");
>>>> +	html("<div class=\"blame_highlight\"> <pre><code>");
>>>
>>> No need for a space before <pre> here.
>>>
>>>>   	if (ctx.repo->source_filter) {
>>>>   		char *filter_arg = xstrdup(basename);
>>>>   		cgit_open_filter(ctx.repo->source_filter, filter_arg);
>>>> @@ -207,7 +207,7 @@ static void print_object(const struct object_id
>>> *oid, const char *path,
>>>>   		html_txt(buf);
>>>>   	}
>>>>   
>>>> -	html("</code></pre>");
>>>> +	html("</code></pre></div>");
>>>>   
>>>>   	html("</div></td>\n");
>>>>   
>> _______________________________________________
>> CGit mailing list
>> CGit at lists.zx2c4.com
>> https://lists.zx2c4.com/mailman/listinfo/cgit