[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#764178: debsources: infobox CSS alignment problem with short files

Hello Stefano,

On 04/11/14 04:44 AM, Stefano Zacchiroli wrote:
> First of all: thanks a lot for your patch, Jason!
> On Fri, Oct 31, 2014 at 07:42:23AM -0400, Jason Pleau wrote:
>> From 6cc9f15d51dd35a5afb82a2c3680e3e5dfc0f93b Mon Sep 17 00:00:00 2001
>> From: Jason Pleau <jason@jpleau.ca>
>> Date: Fri, 31 Oct 2014 00:05:26 -0400
>> Subject: [PATCH] source_file: fix text overlapping the infobox
> I'm no CSS expert, so I'm unable to comment on your patch at the
> moment. Matthieu: can you have a look and comment on Jason's approach at
> fixing #764178.
> I've a separate comment though:
>> When browsing a file's source on sources.debian.net, if the file
>> didn't contain enough text its content would overlap onto the infobox
>> to the right.
> Your commit message essentially restates the bug report, rather than
> explaining how the corresponding change fixes it. The commit message
> should really do the latter, rather than the former.

Thanks for the feedback ! I admit my commit doesn't explain what I did
to fix the issue I was fixing.

First, I'd like to note that there were two issues in the original post
(I think that's what brought up the confusion)

 * If the file was too short, the "infobox" seemed to expand outside
it's container. This was fixed already in
0ed831b256a91287ebfe63c9a52cbbb76816a293 [1] a few weeks ago.

 * If the file contained only one line with only a few characters (for
example, debian/compat is a one liner containing only one character),
the file content was displayed to the right (see my attached picture:
before.png). If the window browser was small, the text even overlapped
over the infobox.

> Particularly in this case, I see no obvious reason why changing the
> right padding of codetable (an horizontal spacing matter) would fix the
> bug (which seems to be a vertical spacing matter). I'm sure it *does*
> fix the bug, but the commit should explain why it does so, so that even
> CSS illiterates as myself could understand the rationale :-)
> Jason: do you think you can update your patch to do so?

My commit adds a padding-right to make sure that even if the file has
one short line, it's content will be left-aligned. The 450px that I've
written is a bit arbitrary. You can look at it in a more graphical way
in my attachment "after.png". The green part is the padding-right that
I've added.

I have attached a new patch, this time with a more descriptive text,
hopefully it will make more sense !

Thanks !

> Many thanks in advance,
> Cheers.


Jason Pleau

Attachment: before.png
Description: PNG image

Attachment: after.png
Description: PNG image

From 2570734fd05f369b1435be89bc830b00b7209279 Mon Sep 17 00:00:00 2001
From: Jason Pleau <jason@jpleau.ca>
Date: Tue, 4 Nov 2014 18:36:10 -0500
Subject: [PATCH] source_file: fix the text alignment in short files

When browsing a file containing only short lines (for example debian/compat
files), its content was aligned to the right, very close to the infobox.

This commit adds a padding-right to the <pre> element containing the
file's content  allowing it to be aligned to the left even on the problematic
files as described above.
 debsources/app/static/css/source_file.css          | 4 ++++
 debsources/app/templates/source_file_code.inc.html | 2 +-
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/debsources/app/static/css/source_file.css b/debsources/app/static/css/source_file.css
index 58328bc..02ae1e8 100644
--- a/debsources/app/static/css/source_file.css
+++ b/debsources/app/static/css/source_file.css
@@ -39,6 +39,10 @@ License: GNU Affero General Public License, version 3 or above.
+#codetable #codecontainer{
+    padding-right: 450px;
 #codetable #sourceslinenumbers{
     text-align: right;
     border-right: 1px solid black;
diff --git a/debsources/app/templates/source_file_code.inc.html b/debsources/app/templates/source_file_code.inc.html
index 9ea5a88..f0a912d 100644
--- a/debsources/app/templates/source_file_code.inc.html
+++ b/debsources/app/templates/source_file_code.inc.html
@@ -23,7 +23,7 @@
         {%- endfor %}</pre>
-      <pre><code id="sourcecode" class="{% if file_language -%}
+      <pre id="codecontainer"><code id="sourcecode" class="{% if file_language -%}
 					{{ file_language }}{% else %}no-highlight
 					{%- endif %}">{% for (line, highlight) in code -%}
           {% if highlight -%}

Attachment: signature.asc
Description: OpenPGP digital signature

Reply to: