Bug#764178: debsources: infobox CSS alignment problem with short files
On Tue, Nov 04, 2014 at 07:03:38PM -0500, Jason Pleau wrote: 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. Thanks Jason, your explanation makes a lot of sense to me now. And the description in the commit is now good enough to explain what's going on also to a CSS-illiterate like myself :) The 450px that I've written is a bit arbitrary. This is the only remaining aspect that concerns me. It is indeed arbitrary, and I'm pretty sure it will lead to nasty side effects on small screens. I've reviewed the rest of the current CSS files, and it seems to me that all other use cases for large px values are more safe (e.g. input field widths) than the one your patch is introducing. How about using a relative width instead? If CSS gurus are reading this, please advise on what you think is the best solution here. Cheers. -- Stefano Zacchiroli . . . . . . . z...@upsilon.cc . . . . o . . . o . o Maître de conférences . . . . . http://upsilon.cc/zack . . . o . . . o o Former Debian Project Leader . . @zack on identi.ca . . o o o . . . o . « the first rule of tautology club is the first rule of tautology club » -- To UNSUBSCRIBE, email to debian-qa-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org Archive: https://lists.debian.org/20141105091445.ga16...@upsilon.cc
Bug#764178: debsources: infobox CSS alignment problem with short files
Hi, 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. We could instead limit the expansion of the first column, which contains the line numbers, see attachment. Cheers, Christophe From 2bab7e42849fb26333dc0962e7a43f530a290143 Mon Sep 17 00:00:00 2001 From: Christophe Siraut d...@tobald.eu.org Date: Wed, 5 Nov 2014 13:43:28 +0100 Subject: [PATCH] css: limit the linenumbers column width, closes: #764178 --- debsources/app/static/css/source_file.css | 4 1 file changed, 4 insertions(+) diff --git a/debsources/app/static/css/source_file.css b/debsources/app/static/css/source_file.css index 58328bc..c0f3575 100644 --- a/debsources/app/static/css/source_file.css +++ b/debsources/app/static/css/source_file.css @@ -18,6 +18,10 @@ License: GNU Affero General Public License, version 3 or above. padding: 0px; } +#codetable td:first-child { +width: 80px; +} + /* CAPTION */ #file_metadata { -- 2.1.1
Bug#764178: debsources: infobox CSS alignment problem with short files
Hey Christophe On 05/11/14 07:51 AM, Christophe Siraut wrote: Hi, 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. We could instead limit the expansion of the first column, which contains the line numbers, see attachment. Looks like that's a better solution ! I had tested with max-width, it didn't work for me. Should have tested with a simple width :) Thanks Cheers, Christophe -- To UNSUBSCRIBE, email to debian-qa-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org Archive: https://lists.debian.org/545a271b.1060...@jpleau.ca
Bug#764178: debsources: infobox CSS alignment problem with short files
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. 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? Many thanks in advance, Cheers. -- Stefano Zacchiroli . . . . . . . z...@upsilon.cc . . . . o . . . o . o Maître de conférences . . . . . http://upsilon.cc/zack . . . o . . . o o Former Debian Project Leader . . @zack on identi.ca . . o o o . . . o . « the first rule of tautology club is the first rule of tautology club » signature.asc Description: Digital signature
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 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 - precode id=sourcecode class={% if file_language -%} + pre id=codecontainercode 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
Bug#764178: debsources: infobox CSS alignment problem with short files
Hello again, I seem to have included the wrong patch in my previous email, please see the attached file in this email. Sorry for the noise -- Jason Pleau 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 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. This fixes the issue on at least Chromium 38 and Iceweasel 31.2. Closes #764178 --- debsources/app/static/css/source_file.css | 4 1 file changed, 4 insertions(+) diff --git a/debsources/app/static/css/source_file.css b/debsources/app/static/css/source_file.css index 58328bc..b21938b 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 pre{ +padding-right: 450px; +} + #codetable #sourceslinenumbers{ text-align: right; border-right: 1px solid black; -- 2.1.1
Bug#764178: debsources: infobox CSS alignment problem with short files
Hello, I have made this small patch that fixes the text overlapping on the infobox. The other issue (that the box containing the infobox was not big enough) seems to have been fixed by 0ed831b256a91287ebfe63c9a52cbbb76816a293 on October 12th Cheers -- Jason Pleau From 97a3760687e0a369ee4aac3c53bbb6eaf2d69882 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 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. This fixes the issue on at least Chromium 38 and Iceweasel 31.2. --- debsources/app/static/css/source_file.css | 4 1 file changed, 4 insertions(+) diff --git a/debsources/app/static/css/source_file.css b/debsources/app/static/css/source_file.css index 58328bc..a70aa30 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 pre{ +padding-right: 50px; +} + #codetable #sourceslinenumbers{ text-align: right; border-right: 1px solid black; -- 2.1.1