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-bugs-dist-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
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-bugs-dist-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
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
Bug#764178: debsources: infobox CSS alignment problem with short files
Package: qa.debian.org Severity: minor User: qa.debian@packages.debian.org Usertags: debsources On Mon, Oct 06, 2014 at 09:21:34AM +0200, Ferenc Wagner wrote: Your wonderful service seems to have alignment issues displaying short files. In the attached screenshot, the file contents (9) is pushed so far to the right that it overlaps with the info box. Please consider fixing this. Thanks Ferenc, I'm forwarding your message to the Debian bug tracking system, so that we don't lose track of your (very reasonable!) request. I suspect the reasonable behavior here would be to ensure that the CSS box showing the file content is always large enough to contain the package info box --- no matter if the infobox is expanded or not, and no matter the amount of lines in the file. Patches from CSS-skilled people that implement such a behavior would be very welcome! 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 »
