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
>> 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.
>
[1]:
http://anonscm.debian.org/cgit/qa/debsources.git/commit/?id=0ed831b256a91287ebfe63c9a52cbbb76816a293
--
Jason Pleau
From 2570734fd05f369b1435be89bc830b00b7209279 Mon Sep 17 00:00:00 2001
From: Jason Pleau
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 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.
/* LINE NUMBERS */
+#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 %}
- {% for (line, highlight) in code -%}
{% if highlight -%}
--
2.1.1
signature.asc
Description: OpenPGP digital signature
