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 <ja...@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
From 2570734fd05f369b1435be89bc830b00b7209279 Mon Sep 17 00:00:00 2001 From: Jason Pleau <ja...@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
signature.asc
Description: OpenPGP digital signature