365247 - Port the attachment table UI used in b.m.o upstream

Status: RESOLVED FIXED

(Bugzilla :: User Interface, enhancement)

Description

[Frédéric Buclin]

I think we all agree to say that the attachment table UI used in b.m.o is nicer than what we currently have in 2.23.3. mkanat and I both think we should take it for 3.0.

Comment 1

[Frédéric Buclin]

Compare https://landfill.bugzilla.org/prodpatches/show_bug.cgi?id=1#a0 and http://landfill.bugzilla.org/qa30/show_bug.cgi?id=1#a0 and you will understand why we cannot release 3.0 without this fix. ;)

Comment 2

[Frédéric Buclin]

Attachment: patch, v1

Compared to the attachment table on b.m.o, this patch doesn't display the number of obsolete attachments (because I don't see why this information is useful), and if there is currently no attachment, the "View All" text is not displayed at all.

Comment 3

[Frédéric Buclin]

Attachment: patch, v1.1

This patch removes a whitespace before a comma and now only displays the MIME type of the attachment if its content has not been deleted. The reason is that all deleted attachments have their MIME type changed to text/plain, so there is no useful information here.

Comment 4

[Olav Vitters]

Comment on attachment 251450 [details] [diff] [review]
patch, v1.1

>Index: template/en/default/attachment/list.html.tmpl

>@@ -73,39 +68,41 @@

>+          <a name="a[% count %]" href="attachment.cgi?id=[% attachment.id %]"
>+             title="View the content of the attachment">
>+            [% attachment.description FILTER html FILTER obsolete(attachment.isobsolete) %]
>+          </a>

IMO you should not link to deleted attachments. Optionally, maybe a good idea to show deleted attachments like obsolete ones (crossed out line in it).

Ideally I'd even like it if the deleted attachments could be hidden by a (renamed?) Hide Obsolete link .

>+            [% IF attachment.datasize %]
>+              ([% attachment.datasize FILTER unitconvert %],
[..]
>+              [% ELSE %]
>+                [% attachment.contenttype FILTER html %])

Should use [%+ or something like to to ensure a space is left after the comma in "$attachment.datasize,".


>Index: skins/standard/global.css

>+#attachment_table th, .bz_attach_footer {
>+    background-color: #E0E0E0;

Nit: I think some validators whine when setting background-color without a font-color (or color?) as well.

Comment 5

[Frédéric Buclin]

Attachment: patch, v2

Linkify the attachment description for non-deleted attachments only + add a missing whitespace before the content type of the attachment.

Comment 6

[Frédéric Buclin]

Checking in skins/standard/global.css;
/cvsroot/mozilla/webtools/bugzilla/skins/standard/global.css,v  <--  global.css
new revision: 1.31; previous revision: 1.30
done
Checking in template/en/default/attachment/list.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/attachment/list.html.tmpl,v  <--  list.html.tmpl
new revision: 1.36; previous revision: 1.35
done

Comment 8

[Max Kanat-Alexander]

Added to the release notes on bug 255155.

Comment 9

[Max Kanat-Alexander]

The correct bug number for those release notes is actually bug 349423.