218569 - Clean up reporting UI

Status: RESOLVED FIXED

(Bugzilla :: Reporting/Charting, defect)

Description

[Gervase Markham [:gerv]]

The reporting tool's been in use for a little while now, and there are a couple
of tweaks I'd like to make to it to simplify the UI.

Gerv

Comment 1

[Gervase Markham [:gerv]]

Attachment: Patch v.1

This patch:

- fixes a problem with spaces in a URL (report.html.tmpl)
- fixes a problem where "Edit report" took you to the wrong sort of edit in
some
  circumstances (report.cgi)
- Reduces the verbiage explaining how this all works, and puts it in the 
  imperative
- Makes the "no vertical axis for pie charts" thing more noticeable by using JS

- Other clarifying wording changes

Gerv

Comment 2

[Jouni Heikniemi]

Comment on attachment 131029 [details] [diff] [review]
Patch v.1

Sorry it took so long, I had a very busy week.

I can't test your patch very well because some of the issues are listed as
"occur in some circumstances", making reproducing them hard without specific
knowledge. But this looks good, apart from the following:

>Index: template/en/default/reports/report.html.tmpl
>===================================================================

>-      <a href="query.cgi?[% switchbase %]&amp;format=report-graph&amp;
>-       chart_format=[% format %]&amp;cumulate=[% cumulate %]">
>+      <a href="query.cgi?[% switchbase %]&amp;chart_format=
>+        [% format %]&amp;format=report-graph&amp;cumulate=[% cumulate %]">

Why this? (I have no problem with it, but please explain anyway)

>Index: template/en/default/search/search-report-graph.html.tmpl
>===================================================================

>+<script><!--

Make that |<script type="text/javascript">|.

>+  [%# The vertical fields are not used for pie charts %]

"The Y axis fields", please - that's more in line with the code terminology and
is less confusing (one might think "vertical" refers to the fields themselves
instead of their function).

>+  function checkPie() {

"checkPie()" sucks :-) "chartTypeChanged()" would allow for more flexibility
and have a better ring in it. Remember to replace all the method calls, too.

For some reason, my IE works oddly with this; the enablement changes are not
drawn when the disabled property is set. I don't know why is this, nor can I
see why the code doesn't work as expected. It's not a big usability loss,
though - the disabling works, just with a small delay.

>+      <noscript><small>(not pie charts)</small><br></noscript>

"Not _for_ pie charts"?


With those comments considered, r=jouni.

Comment 3

[Gervase Markham [:gerv]]

> Why this? (I have no problem with it, but please explain anyway)

Because otherwise the URL gets spaces into it; rearranging it means they are
CHOMPed away.

Requesting approval (updated patch, with all other issues fixed, coming in just
a sec.)

Gerv

Comment 4

[Gervase Markham [:gerv]]

Attachment: Patch v.2

Comment 5

[Gervase Markham [:gerv]]

Fixed. 

Checking in template/en/default/reports/report.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/reports/report.html.tmpl,v
 <--  report.html.tmpl
new revision: 1.8; previous revision: 1.7
done
Checking in report.cgi;
/cvsroot/mozilla/webtools/bugzilla/report.cgi,v  <--  report.cgi
new revision: 1.20; previous revision: 1.19
done
Checking in template/en/default/search/search-report-graph.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/search/search-report-graph.html.tmpl,v
 <--  search-report-graph.html.tmpl
new revision: 1.6; previous revision: 1.5
done
Checking in template/en/default/search/search-report-table.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/search/search-report-table.html.tmpl,v
 <--  search-report-table.html.tmpl
new revision: 1.7; previous revision: 1.6
done

Gerv