Open Bug 400674 Opened 17 years ago Updated 8 years ago

The Administration link should open a menu with links to separate admin pages

Categories

(Bugzilla :: User Interface, enhancement)

3.1.2
enhancement
Not set
normal

Tracking

()

People

(Reporter: LpSolit, Unassigned)

References

Details

Attachments

(1 file, 5 obsolete files)

When hovering the Admistration page in the footer/header of pages, I would like to see a menu with "Products, Parameters, Users, ...". This would let us save a click and more easily administrate Bugzilla.

I'm not asking for the removal of the Administration link! We replaced all the previous administrative links by this single link on purpose (we don't administrate Bugzilla that often and all the links take a lot of place). But saving one click is nice too.
Summary: Hovering the Administration link should open a frame with links to separate admin pages → Hovering the Administration link should open a menu with links to separate admin pages
pyrzak, is that something you would like to fix?
No longer blocks: 325487
If we're willing to change it from "hover" to clicking then I'm happy to. If you'd like an example why clicking is better than hovering, check out google.com, click more, not hover. If you agree to that Lp, I'm happy to take it.
Status: NEW → ASSIGNED
Assignee: ui → guy.pyrzak
Status: ASSIGNED → NEW
Assignee: guy.pyrzak → jillpf55
Attached patch patch v1 (obsolete) — Splinter Review
Attachment #356868 - Flags: review?(mkanat)
Hey Jill. For UI things, ask pyrzak for review first, and then me once pyrzak has passed it.
Summary: Hovering the Administration link should open a menu with links to separate admin pages → The Administration link should open a menu with links to separate admin pages
Max, why did you change the summary of the bug. The new summary is not what I want. Clicking on the link should still display the admin page. Only hovering it should display a menu.
It is both a web standard and a usability thing to NOT do significant actions on the page when hovering, such as bringing up a big menu. This is because hovering isn't an explicit action it is more of a midus touch thing. Users do not purposefully hover, it is usually something you do when thinking or looking at something, and most people would agree that admin isn't something users "explore" to get to a menu. Hovering is usually used for things like tooltips not menu access.

So while it isn't what you want it is the more standard way of doing things. I do think there should be a link to take users to the normal admin page in case they want to read or get access to some of the sub menu options.
Attachment #356868 - Flags: review?(mkanat) → review-
Comment on attachment 356868 [details] [diff] [review]
patch v1


>+
>+#header #amenu_top li {
>+      background: #929BB1;
>+}
>+
We should try to have a better label then amenu, maybe adminmenu ? just a nit

>Index: skins/standard/global.css
>+    #header #amenu_top li {
>+        list-style-type: none;
>+        background: white;
>+        padding: 5px;
>+        font-size: 85%;
>+    }
>+

>Index: template/en/default/global/common-links.html.tmpl
>-      <li><span class="separator">| </span><a href="admin.cgi">Administration</a></li>
>+      <li id="adminlink[% qs_suffix %]"><span class="separator">| </span><a href="admin.cgi" onclick="javascript:showAdminMenu[% qs_suffix %]();return false;">Administration</a></li>
>     [% END %]

>+[% IF user.login %]
>+  <script type="text/javascript">
>+    [%# Set up administration drop down menu %]
>+    var oMenu[% qs_suffix %];
could we have a better name than oMenu (also a nit)

>+    if (oMenu[% qs_suffix %] == null) {
>+      YAHOO.util.Event.onDOMReady(function () {
>+        oMenu[% qs_suffix %] = new YAHOO.widget.Menu("menu[% qs_suffix %]");
>+        oMenu[% qs_suffix %].addItems([
>+            [% IF user.in_group('tweakparams') %]
>+            { text: "Parameters", url:"editparams.cgi" },
>+            { text: "Default Preferences", url:"editsettings.cgi" },
>+            [% END %]
>+            [% IF user.in_group('editcomponents') %]
>+            { text: "Sanity Check", url:"sanitycheck.cgi" },
>+            [% END %]
>+            [% IF (user.in_group('editusers') || user.can_bless) %]
>+            { text: "Users", url:"editusers.cgi" },
>+            [% END %]
>+            [% IF (Param('useclassification') && user.in_group('editclassifications')) %]
>+            { text: "Classification", url:"editclassifications.cgi" },
>+            [% END %]
>+            [% IF (user.in_group('editcomponents') || user.get_products_by_permission("editcomponents").size) %]
>+            { text: "Products", url:"editproducts.cgi" },
>+            [% END %]
>+            [% IF (user.in_group('editcomponents')) %]
>+            { text: "Flags", url:"editflagtypes.cgi" },
>+            [% END %]
>+            [% IF user.in_group('admin') %]
>+            { text: "Custom Fields", url:"editfields.cgi" },
>+            { text: "Field Values", url:"editvalues.cgi" },
>+            { text: "[% terms.Bug %] Status Workflow", url:"editworkflow.cgi" },
>+            [% END %]
>+            [% IF user.in_group('creategroups') %]
>+            { text: "Groups", url:"editgroups.cgi" },
>+            [% END %]
>+            [% IF user.in_group('editkeywords') %]
>+            { text: "Keywords", url:"editkeywords.cgi" },
>+            [% END %]
>+            [% IF user.in_group('bz_canusewhines') %]
>+            { text: "Whining", url:"editwhines.cgi" },
>+            [% END %]
>+            { text: "More Info", url:"admin.cgi" }
>+        ]);
The javascript in the IF statements should be indented for easier reading in the template


>Index: template/en/default/global/header.html.tmpl
>===================================================================
>RCS file: /cvsroot/mozilla/webtools/bugzilla/template/en/default/global/header.html.tmpl,v
>retrieving revision 1.59
>diff -u -r1.59 header.html.tmpl
>--- template/en/default/global/header.html.tmpl	2 Oct 2008 17:08:04 -0000	1.59
>+++ template/en/default/global/header.html.tmpl	14 Jan 2009 01:30:55 -0000
>@@ -195,6 +195,21 @@
>       </script>
>     [% END %]
> 
>+    [% IF user.login %]
>+      [% IF user.in_group('tweakparams') || user.in_group('editusers')
>+          || user.can_bless
>+          || (Param('useclassification') && user.in_group('editclassifications'))
>+          || user.in_group('editcomponents')
>+          || user.in_group('admin') || user.in_group('creategroups')
>+          || user.in_group('editkeywords') || user.in_group('bz_canusewhines')
>+          || user.get_products_by_permission("editcomponents").size %]
>+        [%# following needed for admin menu %]
>+        <script src="js/yui/yahoo-dom-event.js" type="text/javascript"></script>
>+        <script src="js/yui/container_core-min.js" type="text/javascript"></script>
>+        <script src="js/yui/menu-min.js" type="text/javascript"></script>
>+      [% END %]
>+    [% END %]
>+
>     [% IF javascript_urls %]
>       [% FOREACH javascript_url = javascript_urls %]
>         <script src="[% javascript_url FILTER html %]" type="text/javascript"></script>

We need to do something here to make sure we are not loading these YUI files twice when they don't need to be. I'm not sure if the browser handles this or not but we should probably remove the urls from javascript URLS if appropriate. The easiest thing to do is make sure we remove them from other files. I know for example that show_bug already uses the events stuff.

Looks pretty good make these changes and I'll do some testing at the same time.
Attached patch patch 2 (obsolete) — Splinter Review
made all requested changes from first review.
Attachment #356868 - Attachment is obsolete: true
Attachment #357025 - Flags: review?(guy.pyrzak)
I want to see the whole admin page when clicking. I don't like the hover thing either, though -- maybe we can find a middle ground with, say, a dropdown widget next to the Administration link?

   Preferences | Administration |v| | Help | Log out

Then clicking the Administration link leads to the admin page, as it does now. The widget opens the drop-down menu.
(In reply to comment #9)
> I want to see the whole admin page when clicking. 
Why do you want the admin page? I'm not saying that it is bad, but I'd like to know reasons before jumping to a solution. Is there something that that is helpful that won't be on the menu?
(In reply to comment #10)
> Why do you want the admin page?

Yes, there is a full description of each page. When you know nothing about Bugzilla, this is a real help.
Oh, and this still has to work without JS (or with JS enabled but to avoid the menu when using Selenium :) ).
Attached patch patch v3 (obsolete) — Splinter Review
Uploaded the wrong patch file
Attachment #357025 - Attachment is obsolete: true
Attachment #357050 - Flags: review?(guy.pyrzak)
Attachment #357025 - Flags: review?(guy.pyrzak)
This does work with JS turned off.  It just goes directly to the admin page.  Also you can still get to that page via the last option in the menu called "More Info"
Attachment #357050 - Attachment is patch: true
Attachment #357050 - Attachment mime type: application/octet-stream → text/plain
I think there should be a link at the bottom of the admin menu that says "More..." or "Get Descriptions" or something that lets you get to the full page if you want to get to the admin page, but I don't think that is the typical use of this menu. Maybe an item that says "full Admin Page" or something... i dunno but I once you know what those items are i doubt you'll need to go to the admin page.
Comment on attachment 357050 [details] [diff] [review]
patch v3

>Index: skins/contrib/Dusk/global.css
>===================================================================
>RCS file: /cvsroot/mozilla/webtools/bugzilla/skins/contrib/Dusk/global.css,v
>retrieving revision 1.5
>diff -u -r1.5 global.css
>--- skins/contrib/Dusk/global.css	29 Nov 2008 22:08:50 -0000	1.5
>+++ skins/contrib/Dusk/global.css	14 Jan 2009 22:04:50 -0000
>@@ -46,6 +46,14 @@
>     color: white;
> }
> 
>+#header .bd ul {
>+      border: 0;
>+}
>+
>+#header #adminmenu_top li {
>+      background: #929BB1;
>+}
>+
Spacing is off here

> /* body */
> 
> #bugzilla-body {
>@@ -208,6 +216,14 @@
>     padding-bottom: 2ex;
> }
> 
>+#footer .bd ul {
>+      border: 0;
>+}
>+
>+#footer #adminmenu_bottom li {
>+      background: #929BB1;
>+}
>+
here too!

>+       
>+    #footer #adminmenu_bottom li {
>+        list-style-type: none;
>+        background: white;
>+        padding: 5px;
>+        font-size: 95%;
>+    }
Bugzilla prefers to use em instead of px. I'll double check with max about this though.

>--- template/en/default/global/common-links.html.tmpl	5 Nov 2008 17:45:16 -0000	1.18
>+++ template/en/default/global/common-links.html.tmpl	14 Jan 2009 22:04:50 -0000
>@@ -61,7 +61,7 @@
>           || user.in_group('editcomponents') || user.in_group('admin') || user.in_group('creategroups')
>           || user.in_group('editkeywords') || user.in_group('bz_canusewhines')
>           || user.get_products_by_permission("editcomponents").size %]
>-      <li><span class="separator">| </span><a href="admin.cgi">Administration</a></li>
>+      <li id="adminlink[% qs_suffix %]"><span class="separator">| </span><a href="admin.cgi" onclick="javascript:showAdminMenu[% qs_suffix %]();return false;">Administration</a></li>
>     [% END %]
I don't think you need the javascript: here for the onclick. But i don't think it is wrong
> 
>     [% PROCESS link_to_documentation %]
>@@ -118,6 +118,8 @@
>   [% END %]
> </ul>
> [% Hook.process("link-row") %]
>+<div id="adminmenu[% qs_suffix %]">
>+</div>
> [% BLOCK link_to_documentation %]
>     [% IF doc_section && Param('docs_urlbase') %]
>       <li>
>@@ -126,3 +128,69 @@
>       </li>
>     [% END %]
> [% END %]

>===================================================================
>RCS file: /cvsroot/mozilla/webtools/bugzilla/template/en/default/global/header.html.tmpl,v
>retrieving revision 1.59
>diff -u -r1.59 header.html.tmpl
>--- template/en/default/global/header.html.tmpl	2 Oct 2008 17:08:04 -0000	1.59
>+++ template/en/default/global/header.html.tmpl	14 Jan 2009 22:04:50 -0000
>@@ -195,6 +195,27 @@
>       </script>
>     [% END %]
> 
>+    [% IF user.login %]
>+      [% IF user.in_group('tweakparams') || user.in_group('editusers')
>+          || user.can_bless
>+          || (Param('useclassification') && user.in_group('editclassifications'))
>+          || user.in_group('editcomponents')
>+          || user.in_group('admin') || user.in_group('creategroups')
>+          || user.in_group('editkeywords') || user.in_group('bz_canusewhines')
>+          || user.get_products_by_permission("editcomponents").size %]
>+        [%# following needed for admin menu %]
>+        [% IF !javascript_urls.grep('yahoo-dom-event').size %]
>+            <script src="js/yui/yahoo-dom-event.js" type="text/javascript"></script>
>+        [% END %]
>+        [% IF !javascript_urls.grep('container-core-min').size %]
>+            <script src="js/yui/container_core-min.js" type="text/javascript"></script>
>+        [% END %]
>+        [% IF !javascript_urls.grep('menu-min').size %]
>+            <script src="js/yui/menu-min.js" type="text/javascript"></script>
>+        [% END %]
>+      [% END %]
>+    [% END %]
>+
>     [% IF javascript_urls %]
>       [% FOREACH javascript_url = javascript_urls %]
>         <script src="[% javascript_url FILTER html %]" type="text/javascript"></script>
>--- /dev/null	2009-01-14 13:40:03.000000000 -0800
About the YUI version...

Bugzilla has updated it's YUI version so we need to do that as well
Attachment #357050 - Flags: review?(guy.pyrzak) → review-
> Bugzilla prefers to use em instead of px. I'll double check with max about
> this though.

Yes, px doesn't mix well with different browsers, personal style sheets, personal font sizes, you name it.
Attached patch patch v4 (obsolete) — Splinter Review
fixed the spacing and updated yui/menu-min.js to version 2.6.0
Attachment #357050 - Attachment is obsolete: true
Attachment #360130 - Flags: review?(guy.pyrzak)
Please resubmit your patch with at least 3 lines of context.
Attached patch patch v5 (obsolete) — Splinter Review
Admin link is now a menu.  To get the descriptions of the menu items, the user can select "More Info" in the menu.
Attachment #360130 - Attachment is obsolete: true
Attachment #372440 - Flags: review?(LpSolit)
Attachment #360130 - Flags: review?(guy.pyrzak)
Attachment #372440 - Attachment is patch: true
Attachment #372440 - Flags: review?(LpSolit) → review?(guy.pyrzak)
Attachment #372440 - Flags: review-
Comment on attachment 372440 [details] [diff] [review]
patch v5

menu-min.js is still YUI 2.3.1.

Also, I do hope we went with "mouseover opens the menu, click opens the Administration screen" although I haven't seen the actual UI in this patch so I could possibly be convinced otherwise.
Attached patch updated yui libSplinter Review
This patch has version "2.6.0" for the yui js files.

Hover does not show the menu.  Clicking does.  That's what google.com does and yahoo.com.  Plus it leaves open the option for tooltips.
Attachment #372440 - Attachment is obsolete: true
Attachment #372677 - Flags: review?(guy.pyrzak)
Attachment #372440 - Flags: review?(guy.pyrzak)
Attachment #372677 - Flags: review?(guy.pyrzak) → review-
Comment on attachment 372677 [details] [diff] [review]
updated yui lib

The menu appears empty (contains no visible text) in Firefox 3.6.2. I didn't test other browsers.
  I think since they're links, and not boxes, it's OK for click to show the menu, and not hover. If they were boxes, I'd expect hover to show them menu. I haven't looked at the patch, but there should be a link at the top or bottom of the list to open the normal admin.cgi, if we do click-to-show.
No longer blocks: ajax
Depends on: bz-pretty
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.
Target Milestone: Bugzilla 4.4 → Bugzilla 5.0
Assignee: jillpf55 → ui
Target Milestone: Bugzilla 5.0 → ---
You need to log in before you can comment on or make changes to this bug.