276560 - We should use webbadge icons for alternate format links on buglist and reports

Status: RESOLVED FIXED

(Bugzilla :: Query/Bug List, enhancement, P1)

Description

[Dave Miller [:justdave]]

What prompted me to file this is the pile of links at the bottom of the buglist
page are starting to get a little cluttered.  I was trying to think of a way to
clean them up, and realized that several of the links are now providing
alternate means of viewing the buglist (CSV, iCal, etc).  Many websites have
little orange buttons with the format name on them or similar.  Wouldn't
necessarily need to be orange, and I can think of better-looking ways to do it,
but making all of the alternate format links be icons that all match each other
and describe the content would be a cool way to distinguish the alternate format
links from the action links.

Comment 1

[Dave Miller [:justdave]]

Reassigning bugs that I'm not actively working on to the default component owner
in order to try to make some sanity out of my personal buglist.  This doesn't
mean the bug isn't being dealt with, just that I'm not the one doing it.  If you
are dealing with this bug, please assign it to yourself.

Comment 2

[Frédéric Buclin]

This is becoming really critical. The UI is becoming totally ugly (screenshot coming).

Comment 3

[Frédéric Buclin]

Attachment: screenshot

See how ugly things now are. We must really improve this for 4.4.

Comment 4

[Frédéric Buclin]

We are going to branch for Bugzilla 4.4 next week and this bug is too invasive or too risky to be accepted for 4.4 at this point. The target milestone is set to 5.0 which is our next major release.

I ask the assignee to reassign the bug to the default assignee if you don't plan to work on this bug in the near future, to make it clearer which bugs should be fixed by someone else on time for 5.0.

Comment 5

[Frédéric Buclin]

Ishitva said he wanted to work on it.

Comment 6

[Ishitva Goel]

Attachment: patch.diff

The patch replaces the button XML and the links CSV ,FEED and ICAL with images , and the Change Columns , and edit search link with a button . Alignment is also corrected .

Comment 7

[Ishitva Goel]

Attachment: RSS_icon.png

This is an image for the FEED link . This needs to be named as RSS_icon.png and put inside the bugzilla/images folder .

Comment 8

[Ishitva Goel]

Attachment: ICAL_icon.png

This is an image for the ICAL link . This needs to be named as ICAL_icon.png and put inside the bugzilla/images folder .
This image has been taken from https://www.iconfinder.com/icons/48653/calendar_icon#size=32

Comment 9

[Ishitva Goel]

Attachment: XML_icon.png

This image replaces the XML button . It should be named as XML_icon.png and put inside the bugzilla/images folder .

Comment 10

[Ishitva Goel]

Attachment: CSV_icon.png

This image replaces CSV link . It should be named as CSV_icon.png and put inside the bugzilla/images folder .

Comment 11

[Ishitva Goel]

Attachment: patch2.diff

This patch adds Title and Alt attributes to the images .

Comment 12

[Frédéric Buclin]

Comment on attachment 8382022 [details]
RSS_icon.png

We already have this icon in images/rss.png. No need to duplicate it.

Comment 13

[Dave Miller [:justdave]]

(In reply to Frédéric Buclin from comment #12)
> We already have this icon in images/rss.png. No need to duplicate it.

but that image is only 16x16, and it looks funny upscaled.  Needs a 24x24 icon to fit in with the other buttons here.

Comment 14

[Ishitva Goel]

Attachment: patch6.diff

Updated Patch : Removed whitespaces and put minor fixes .

Comment 15

[Ishitva Goel]

Attachment: patch8.diff

Comment 16

[Ishitva Goel]

Attachment: patch9.diff

Removed the unrelated changes from the patch .

Comment 17

[Dave Miller [:justdave]]

Comment on attachment 8382022 [details]
RSS_icon.png

Should name this rss.png and replace the existing one in images named that (which is smaller).  You'll need to double-check the one on the front page to make sure it either looks good at 24p or scales down correctly.

For citation purposes, this image was generated at 24p from the svg file provided by feedicons.com

Comment 18

[Dave Miller [:justdave]]

Comment on attachment 8382024 [details]
ICAL_icon.png

for citation purposes, this icon came from http://www.easyicon.net/language.en/504792-calendar_icon.html which lists it licensed as "free for commercial use"

Comment 19

[Dave Miller [:justdave]]

Comment on attachment 8382029 [details]
XML_icon.png

This icon was created by me, attempting to reproduce the shape and gradient from the RSS icon, using Futura Ultra-Bold Condensed at 11pt for the font.  Retargetting review to Gerv for license verification purposes.  That font is apparently a commercial font, but it ships with OS X.  Is it a problem to have graphics created with it, or does that only matter if we're actually embedding the font?

Comment 20

[Dave Miller [:justdave]]

Comment on attachment 8382031 [details]
CSV_icon.png

This icon was created by me, attempting to reproduce the shape and gradient from the RSS icon, using Futura Ultra-Bold Condensed at 11pt for the font.  Retargetting review to Gerv for license verification purposes.  That font is apparently a commercial font, but it ships with OS X.  Is it a problem to have graphics created with it, or does that only matter if we're actually embedding the font?

Comment 21

[Ishitva Goel]

In the latest patch , all the images have been renamed and used inside the list.html.tmpl file as :

1)RSS_icon.png -> rss.png
2)CSV_icon.png -> csv.png
3)XML_icon.png -> xml.png
4)ICAL_icon.png -> ical.png

Comment 22

[Gervase Markham [:gerv]]

Comment on attachment 8382029 [details]
XML_icon.png

There is no problem making graphics using a rasterization of a commercial font.

Gerv

Comment 23

[Gervase Markham [:gerv]]

Comment on attachment 8382031 [details]
CSV_icon.png

There is no problem making graphics using a rasterization of a commercial font.

Gerv

Comment 24

[Frédéric Buclin]

Comment on attachment 8382327 [details] [diff] [review]
patch9.diff

>=== modified file 'template/en/default/list/list.html.tmpl'

>+               <img src="/images/xml.png" width="24" height="24" alt="XML" title="XML Format" >

The URL pointing to icons are incorrect. The leading / must go away, or the web server is unable to find them.


>       <div class="bz_query_edit">
>-        <a href="[% PROCESS edit_search_url %]">Edit&nbsp;Search</a>

"Edit search" and other related actions should be on their own line instead of wrapping as they do currently. Also, for consistency, the "Change several bugs at once" and "send mail to bug assignees" should also be buttons, else the UI is very inconsistent.

Another problem with this patch is that you replaced the small rss.png icon with a bigger one, breaking the UI on index.cgi. Maybe the old icon should be rename to rss_small.png and be used there.


I didn't read your code nor tested it further, so maybe more comments will come later. :)

Comment 25

[Ishitva Goel]

Can we put <div class="bz_query_edit"> and <div class="bz_query_remember"> inside a <p>..</p> to make the "Edit Search" button and  "Remember Search as " button to get displayed on the next line ? 
It gives the effect as in this image http://imagebin.org/297315. 
Also I have noticed that a small connecting line is coming between the buttons and images inside the <div class="bz_query_links"> which wasn't coming earlier , how can we remove that ?

Comment 26

[Ishitva Goel]

Attachment: patch15.diff

According to the previous review , I have 
1) Corrected the image URLs 
2) Placed "Edit Search " and "Remember Search as" options in a <p> tag to place them on seperate line . 
3)"Change several bugs at once" and "send mail to bug assignees" are now buttons 
4)the original rss icon which was being used is renamed as rss_small.png and the standard/global.css file has been updated with this name .

Comment 27

[Ishitva Goel]

Edit in above Comment 26 2nd point : 
<br> is used in place of <p> tag .

Comment 28

[Gervase Markham [:gerv]]

Comment on attachment 8386069 [details] [diff] [review]
patch15.diff

For me, at least, some of the buttons have a different font size to other ones - although I can't immediately see why in Inspector. Also, is it possible to produce a patch which includes the PNG files? (Particularly now we've switched to git?)

Gerv

Comment 29

[Ishitva Goel]

Attachment: patch15a.diff

According to the previous review , in this patch 

1)Font sizes have been made similar .
2)The lines which were coming between the icons and buttons of the div bz_query_links have been removed . They were coming because of whitespaces between the <a> tags .

Comment 30

[Frédéric Buclin]

Comment on attachment 8390633 [details] [diff] [review]
patch15a.diff

Dont write:

<foo
><bar></foo>

That's ugly and doesn't make sense. Write.

<foo>
  <bar>
</foo>

instead.

Comment 31

[Gervase Markham [:gerv]]

LpSolit: I believe he did that to eliminate the whitespace between <a> and <button>, which caused random bits of blue underlining to appear.

Gerv

Comment 32

[Frédéric Buclin]

Attachment: screenshot for patch15

(In reply to Gervase Markham [:gerv] from comment #31)
> LpSolit: I believe he did that to eliminate the whitespace between <a> and
> <button>, which caused random bits of blue underlining to appear.

A simple .bz_query_links a { text-decoration: none; } would fix this problem.


Note that patch15.diff causes this broken UI, see the screenshot.

Comment 33

[Ishitva Goel]

Lpsolit : The broken UI can be been fixed by adding the rule (in buglist.css) :-

.buglist_menu .bz_query_buttons {
    display: inline;
}

I need further instructions on how to fix the problem of the tiny lines coming between the buttons and the icons of the div .bz_query_links . As you suggest we can  use the rule { text-decoration: none; } and it works fine , but glob prefers the HTML to be fixed . So can we decide together which approach should be implemented .

Ishitva

Comment 34

[:glob ✱]

(In reply to Ishitva Goel from comment #33)
> I need further instructions on how to fix the problem of the tiny lines
> coming between the buttons and the icons of the div .bz_query_links . As you
> suggest we can  use the rule { text-decoration: none; }

can you please do both.

we need text-decoration:none to remove underlining of images, and the anchors should not encapsulate extraneous whitespace.

Comment 35

[Ishitva Goel]

Attachment: patch17a.diff

This patch :
1)Fixes the broken UI by adding the rule 
.buglist_menu .bz_query_buttons { display: inline;}

2)Adds the rule .bz_query_links a { text-decoration: none; } to buglist.css , to solve the problem of tiny lines coming up.

3)Removes the extraneous white spaces from in between the anchor tags .

Comment 36

[Frédéric Buclin]

Comment on attachment 8392070 [details] [diff] [review]
patch17a.diff

>+           ><img title="CSV Format" alt="CSV" src="images/csv.png" height="24" width="24"
>+        ></a>

There is no reason to move ></a> on its own line. Simply move it on the previous line.

I still think this way of writing code is ugly and totally useless.

Comment 37

[Ishitva Goel]

Attachment: patch17b.diff

This patch does formatting changes to the code in list.html.tmpl .

Comment 38

[Ishitva Goel]

Attachment: patch17d.diff

Fixed the image truncation on Chrome and IE11.

Comment 39

[Frédéric Buclin]

Comment on attachment 8395259 [details] [diff] [review]
patch17d.diff

>=== modified file 'template/en/default/list/list.html.tmpl'

>+        <form method="post" action="show_bug.cgi">
>+            <input type="hidden" name="ctype" value="xml">

The indentation in templates is 2 characters, not 4 (the bad indentation was already there).


>+          [% END %]
>+            <input type="hidden" name="excludefield" value="attachmentdata">

No reason to ident after a [% END %].


>+        ><img title="ICAL Format" alt="ICAL" src="images/ical.png" height="24" width="24"></a>

ICAL should be iCal.


>+        ><button>Change&nbsp;Columns</button></a>

There is no need to keep &nbsp;. The text in buttons won't wrap unless the screen is less wide than the button itself. Else the whole button will wrap. So they can be removed (which will also make the code easier to read).


Otherwise looks good and works fine. r=LpSolit with these changes on checkin.

Comment 40

[Frédéric Buclin]

To ssh://[e-mail]/bugzilla/bugzilla.git
   d6370f0..d9cd470  master -> master