619826 - implement navigation links

Status: RESOLVED FIXED

(Tree Management Graveyard :: OrangeFactor, enhancement)

Description

[Jeff Hammel]

Currently, there is no clear way of navigating between views.  There
should be an e.g. menubar that both shows what views are available and
gives links to each of these views, possibly with tooltips.

Comment 1

[Jeff Hammel]

see also bug 619833

Comment 2

[Jeff Hammel]

This serves as a list of the different views.  In addition, the as-yet nonexistent about page should display the various query strings (API) associated with each view and describe how the views work.

Comment 3

[Jeff Hammel]

A bug number text box should also be part of the navigation which will take you to the ?display=Bug view

Comment 4

[Mark Côté [:mcote]]

Attachment: Menu and association changes

There is now a menu in the top-right corner with the following links: Heat Map, Bug Details, Test Run, Top Bugs, Simulator, About (some of these will change in other bugs).

I was thinking of making it fixed, but it overlaps long titles and doesn't look good, so it's a right float, and the rest of the divs start underneath it.

I ripped out the about sidebar and made "display=About" load it in the "display" div.  This required futzing with the CSS, so while I was at it I ripped out the calendar as well.  This breaks the single-day Heat Map in that the only way to get there, AFAIK, was through the calendar.  We will resurrect this when we add a separate view (bug 620045) with a calendar button (bug 609986).

Since it doesn't work without parameters, I added defaults to displayTestRun().  Putting all dropdowns to "All" was really slow, so I made the platform dropdown default to the top value (atm, Fedora) and left the others at "All", which is a fair bit faster.

The bug view also doesn't work with defaults, but it's hard to know what bug to display by default (top bug would take too long to load IMHO).  So it defaults to a blank page with a text-entry box instead of a bug number.  You enter a bug number and hit enter and it will load that bug's page.  Along those lines, you can now click on the bug number on a regular Bug Details view, which will cause it to turn into a text-entry box, so you can navigate to a different, arbitrary bug.  I put a little pencil icon to make that feature more obvious.

All told, in addition to this bug, this patch fixes bug 615593, bug 619171 (you have to go to "Bug Details" first, but I think that's close enough), the main part of bug 619833, and bug 620010.  I'll resolve and/or comment on them as appropriate once my patch is reviewed & committed.

We will probably be combining the Test Run view with the Heat Map view (bug 620046), but for now I left it in, as I think this patch does quite enough on its own.

Comment 5

[Joel Maher ( :jmaher ) (UTC -8)]

Comment on attachment 499082 [details] [diff] [review]
Menu and association changes

>diff --git a/_attachments/orange.html b/_attachments/orange.html
>--- a/_attachments/orange.html
>+++ b/_attachments/orange.html
>       var mplat = false;
>       if (plat == '' || plat == 'All') {
>-        mplat = true;;
>+        mplat = true;

seriously a ;; in the existing code, I blame a monkey!


>@@ -159,16 +169,32 @@
>   function displayTestRun(app, id, titleid, args) {
>+      if (plat == "" || plat === undefined) {
>+        plat = platforms[getKeys(platforms)[0]];
>+      }

I like this idea in general, but I fear errors here.  We should do some kind of bounds checking to ensure platforms is defined.
if (plat == "" || plat === undefined) {
  if (platforms != null && platforms != undefined && len(platforms) >= 1) {
    plat = platforms[getKeys(platforms)[0]];
  } else {
    plat = "All";
  }
}

>       if (startday == "" || startday === undefined) {
>         startday = '';
>       }

should we make startday = today-30days?


>diff --git a/_attachments/scripts/woo.bugs.js b/_attachments/scripts/woo.bugs.js
>--- a/_attachments/scripts/woo.bugs.js
>+++ b/_attachments/scripts/woo.bugs.js
>@@ -119,23 +119,42 @@
>+  function displayBugEntry(selector) {
>+    selector.html('<form id="bugidform"><input type="text" size="8" id="bugidentry"/></form>');
>+    $("#bugidform").submit(function() {
>+      window.location.assign(buildUrl("Bug", {bugid: $("#bugidentry").val() }));
>+      return false;
>+    });
>+  }

you are cool!  on a related but unrelated note, how can we get the url to accept >1 bug id?


r=me with the platform checking and startday comments mentioned above addressed.

Comment 6

[Mark Côté [:mcote]]

(In reply to comment #5)
> Comment on attachment 499082 [details] [diff] [review]
> Menu and association changes
> 
> >@@ -159,16 +169,32 @@
> >   function displayTestRun(app, id, titleid, args) {
> >+      if (plat == "" || plat === undefined) {
> >+        plat = platforms[getKeys(platforms)[0]];
> >+      }
> 
> I like this idea in general, but I fear errors here.  We should do some kind of
> bounds checking to ensure platforms is defined.
> if (plat == "" || plat === undefined) {
>   if (platforms != null && platforms != undefined && len(platforms) >= 1) {
>     plat = platforms[getKeys(platforms)[0]];
>   } else {
>     plat = "All";
>   }
> }

Okay that doesn't work well if 'platforms' is not defined.  I'm gonna use this:

      if (plat == "" || plat === undefined) {
        try {
          if (getKeys(platforms).length > 0) {
            plat = platforms[getKeys(platforms)[0]];
          } else {
            plat = "All";
          }
        } catch (e) {
          plat = "All";
        }
      }

Slightly redundant, but accessing 'platforms' if it not defined gives an exception, whereas accessing a nonexistent key of platforms will just result in plat being assigned 'undefined' as value, so we need both kinds of checks.

Oh and pretty sure nothing else works if platforms is undefined or empty, even after it gets past this part. :P

> >       if (startday == "" || startday === undefined) {
> >         startday = '';
> >       }
> 
> should we make startday = today-30days?

Bug 620023 was created for this (well, 3 months instead of 30 days) on the Orange Factor view.  I commented on it that TestRun view should be fixed at the same time.  I think this patch does enough. :)

> you are cool!  on a related but unrelated note, how can we get the url to
> accept >1 bug id?

Hm well not entirely sure that this view would make sense for multiple bugs... we'd at least have to change it a little bit.

But if we do want this (for this or any other view), I think it should work like bzAPI does: a comma-separated list.  So "display=Bug&bugid=X,Y,Z".  As long as we strip out spaces when processing, this'll make it easy and intuitive for users to enter it into text-entry boxes.

Comment 7

[Jeff Hammel]

(In reply to comment #6)
> (In reply to comment #5)

> But if we do want this (for this or any other view), I think it should work
> like bzAPI does: a comma-separated list.  So "display=Bug&bugid=X,Y,Z".  As
> long as we strip out spaces when processing, this'll make it easy and intuitive
> for users to enter it into text-entry boxes.

i'm not sure if it makes sense for multiple bugs either.  i'd be more a fan of repeating the query string in the traditional http manner: display=Bug&bugid=X&bugid=Y&bugid=Z

Comment 8

[Mark Côté [:mcote]]

Either one.  Either we do comma-processing in the display*() functions or in the form submission function; doesn't really matter.

Comment 9

[Mark Côté [:mcote]]

Committed: http://hg.mozilla.org/automation/orangefactor/rev/91df4dfdb70c