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. > [1]: http://anonscm.debian.org/cgit/qa/debsources.git/commit/?id=0ed831b256a91287ebfe63c9a52cbbb76816a293 -- 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.
/* 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 %}</pre>
</td>
<td>
- <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 -%}
--
2.1.1
Attachment:
signature.asc
Description: OpenPGP digital signature