* [PATCH] css: make highlight div absolute and top left @ 2018-06-18 5:26 andy 2018-06-18 6:02 ` [PATCH v2] blame: css: make blame " andy 0 siblings, 1 reply; 6+ messages in thread From: andy @ 2018-06-18 5:26 UTC (permalink / raw) 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 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. Signed-off-by: Andy Green <andy at warmcat.com> --- cgit.css | 2 ++ 1 file changed, 2 insertions(+) diff --git a/cgit.css b/cgit.css index da8d9b0..a186ba9 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.highlight { position: absolute; top: 0; left: 0; } + div#cgit table.list th { font-weight: bold; /* color: #888; ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2] blame: css: make blame highlight div absolute and top left 2018-06-18 5:26 [PATCH] css: make highlight div absolute and top left andy @ 2018-06-18 6:02 ` andy 2018-06-18 18:57 ` john 0 siblings, 1 reply; 6+ messages in thread From: andy @ 2018-06-18 6:02 UTC (permalink / raw) 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. "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; } + 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>"); 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"); ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2] blame: css: make blame highlight div absolute and top left 2018-06-18 6:02 ` [PATCH v2] blame: css: make blame " andy @ 2018-06-18 18:57 ` john 2018-06-18 19:11 ` andy 0 siblings, 1 reply; 6+ messages in thread From: john @ 2018-06-18 18:57 UTC (permalink / raw) 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.) > "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. > + > 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"); > ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2] blame: css: make blame highlight div absolute and top left 2018-06-18 18:57 ` john @ 2018-06-18 19:11 ` andy 2018-06-19 19:59 ` john 0 siblings, 1 reply; 6+ messages in thread From: andy @ 2018-06-18 19:11 UTC (permalink / raw) 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. 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"); >> ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2] blame: css: make blame highlight div absolute and top left 2018-06-18 19:11 ` andy @ 2018-06-19 19:59 ` john 2018-06-20 0:50 ` andy 0 siblings, 1 reply; 6+ messages in thread From: john @ 2018-06-19 19:59 UTC (permalink / raw) 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? [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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2] blame: css: make blame highlight div absolute and top left 2018-06-19 19:59 ` john @ 2018-06-20 0:50 ` andy 0 siblings, 0 replies; 6+ messages in thread From: andy @ 2018-06-20 0:50 UTC (permalink / raw) 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-06-20 0:50 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-06-18 5:26 [PATCH] css: make highlight div absolute and top left andy 2018-06-18 6:02 ` [PATCH v2] blame: css: make blame " andy 2018-06-18 18:57 ` john 2018-06-18 19:11 ` andy 2018-06-19 19:59 ` john 2018-06-20 0:50 ` andy
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).