Closed Bug 376673 (bz-simple-enter) Opened 16 years ago Closed 14 years ago

Add a simple bug entry form

Categories

(Bugzilla :: Creating/Changing Bugs, enhancement, P1)

enhancement

Tracking

()

RESOLVED FIXED
Bugzilla 3.4

People

(Reporter: LpSolit, Assigned: mkanat)

References

Details

(Whiteboard: [survey-important])

Attachments

(3 files, 5 obsolete files)

Attached patch patch, v1 (obsolete) — Splinter Review
This will be the default for all users, especially for newbies who won't have to deal with tens of cryptic fields. You can decide what to display by default using a user pref, and in all cases can you toggle between the simple and full forms using the link provided in the form itself.
Attachment #260784 - Flags: review?(wicked+bz)
Attachment #260784 - Flags: review?(mkanat)
Attached patch patch, v1.1 (obsolete) — Splinter Review
IE6 doesn't understand 'table-row-group', as usual. Now works with Firefox, Opera and IE6. No other change.
Attachment #260784 - Attachment is obsolete: true
Attachment #260786 - Flags: review?(wicked+bz)
Attachment #260786 - Flags: review?(mkanat)
Attachment #260784 - Flags: review?(wicked+bz)
Attachment #260784 - Flags: review?(mkanat)
Attached patch patch, v1.2 (obsolete) — Splinter Review
Fix bitrot due to another checkin I did meanwhile.
Attachment #260786 - Attachment is obsolete: true
Attachment #261169 - Flags: review?(wicked+bz)
Attachment #261169 - Flags: review?(mkanat)
Attachment #260786 - Flags: review?(wicked+bz)
Attachment #260786 - Flags: review?(mkanat)
Comment on attachment 261169 [details] [diff] [review]
patch, v1.2

>Index: skins/standard/global.css
>+    margin: 0 0.5em;

  Nit: The three-digit version of "margin" is always confusing to me. Could we just make it two digits or four digits?

>+  function toggle_display(link) {

  Instead of this, we should use the modified TUI.js that I'm using for show_bug.cgi. Trust me, it's much cooler, because it already uses cookies.

  The HTML all looks fine, though.
Attachment #261169 - Flags: review?(wicked+bz)
Attachment #261169 - Flags: review?(mkanat)
Attachment #261169 - Flags: review-
Depends on: 373418
No longer blocks: 57842
Depends on: 57842
Priority: -- → P1
Blocks: tooformy
Bugzilla 3.2 is now frozen. Only enhancements blocking 3.2 or specifically approved for 3.2 may be checked in to the 3.2 branch. If you would like to nominate your enhancement for Bugzilla 3.2, set "blocking3.2" tp "?", and either the target milestone will be changed back, or the blocking3.2 flag will be granted, if we will accept this enhancement for Bugzilla 3.2.
Target Milestone: Bugzilla 3.2 → Bugzilla 4.0
I can finish this patch off if you'd like.
Sure, if you want to.
This was one of the most common issues reported during the Bugzilla survey. This should pretty much be considered our highest-priority UI issue. If I could add a keyword or set the priority higher than P1, I would.
Whiteboard: [survey-important]
Reassigning to pyrzak, per comment 5.
Assignee: LpSolit → guy.pyrzak
Assignee: guy.pyrzak → mkanat
No longer depends on: 373418
Target Milestone: Bugzilla 4.0 → Bugzilla 3.4
Alias: bz-simple-enter
Depends on: 473677
Attached patch v2 (obsolete) — Splinter Review
Okay, here's a completely-rewritten patch. This uses YUI, and it saves the state of whether or not the advanced fields are shown in a cookie. By default, it hides the advanced fields, so users get the simple form unless they ask for the advanced form. However, once you click "Show Advanced Fields" every page load will show the advanced fields (on this machine) until you decide to hide them (at which point all future enter_bug loads will have the simple form, etc.)

I will upload screenshots, also.

LpSolit--I think it's safe to assume that the JS is fine, I don't think we have anybody available who can do JS reviews, and I'm just using YUI everywhere, so it should work fine across all browsers.
Attachment #261169 - Attachment is obsolete: true
Attachment #357080 - Flags: review?(guy.pyrzak)
Attachment #357080 - Flags: review?(LpSolit)
Oh, and it works fine with JS disabled, too--in that case the advanced fields are always shown.
Here is what the page looks like by default now, with the advanced fields hidden. (And it will be even simpler if you don't have groups in your Bugzilla, which is a common case.)
And here's what happens when you click "Show Advanced Fields".
I haven't gotten to look at the code but i REALLY like the screen shots, nice job max. I'll get to the review soon.
A comment after taking a closer look is I'd like to see the add attachment as part of the simple bug page b/c that is something easy for people to do and isn't too scary. It might be too much to think people would capture screenshots of errors, crash reports, or something like that but I don't think it is as scary or confusing as some of the other fields.
Comment on attachment 357080 [details] [diff] [review]
v2

> 
>+
>+TUI_alternates['expert_fields'] = 'Show Advanced Fields';
>+// Hide the Advanced Fields by default, unless the user has a cookie
>+// that specifies otherwise.
>+TUI_hide_default('expert_fields')
>+
> -->
> </script>
> 
>@@ -187,7 +193,16 @@
>   </tr>
> 
>   <tr>
>-    <td colspan="4">&nbsp;</td>
>+    <td colspan="4">
>+      <script type="text/javascript">
>+      <!--
>+        document.write('<a id="expert_fields_controller" class="controller"'
>+            + ' href="javascript:TUI_toggle_class(\'expert_fields\')">Hide'
>+            + ' Advanced Fields<\/a>');
>+      // -->
>+      </script>
>+      <noscript>&nbsp;</noscript>
>+    </td>
>   </tr>

I dislike using document.write to add elements to the dom. Which is what you're doing here. Instead I prefer to have the dom element hidden and have the JS show it if it works. I also find it easier to read. 

I also don't like it because it can often cause some browsers to get angry (older ones). Plus this requires you to escape single quotes and other messy things which is just annoying. Just use bz_default_hidden.


> 
>   <tr>
>@@ -249,7 +264,7 @@
>       <select name="version" size="5">
>         [%- FOREACH v = version %]
>           <option value="[% v FILTER html %]"
>-            [% ' selected="selected"' IF v == default.version %]>[% v FILTER html -%]
>+           [% ' selected="selected"' IF v == default.version %]>[% v FILTER html -%]
>           </option>
NIT spacing is off here

>         [%- END %]
>       </select>
>@@ -454,7 +469,7 @@
>   </tr>
> </tbody>
>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	15 Jan 2009 02:23:33 -0000
>@@ -188,12 +188,16 @@
>             type="text/css">
>     <![endif]-->
> 
>-
>-    [% IF javascript %]
>-      <script type="text/javascript">
>-        [% javascript %]
>-      </script>
>-    [% END %]
>+    <script type="text/javascript">
>+    <!--
>+        [%# Make some Bugzilla information available to all scripts. %]
>+        var COOKIE_PATH = '[% Param('cookiepath') FILTER js %]';
>+
>+        [% IF javascript %]
>+          [% javascript %]
>+        [% END %]
>+    // -->
>+    </script>
> 
>     [% IF javascript_urls %]
>       [% FOREACH javascript_url = javascript_urls %]
We should be using the bugzilla namespace for this stuff instead of making a global variable.
Attachment #357080 - Flags: review?(guy.pyrzak) → review-
Attachment #357080 - Flags: review?(LpSolit)
(In reply to comment #15)
> We should be using the bugzilla namespace for this stuff instead of making a
> global variable.

  But these are things that should be available even without YUI. We could have a global "bugzilla" object that could be accessed like bugzilla.COOKIE_PATH, perhaps.
Attached patch v3 (obsolete) — Splinter Review
Okay, I fixed everything except the global variables thing. If you want me to change that still, I can, we should just decide how to do it.
Attachment #357080 - Attachment is obsolete: true
Attachment #358435 - Flags: review?(guy.pyrzak)
Comment on attachment 358435 [details] [diff] [review]
v3


>+function _TUI_store(aClass, state) {
>+    YAHOO.util.Cookie.setSub(TUI_COOKIE_NAME, aClass, state,
>+    {
>+        expires: new Date('January 1, 2038'),
>+        path: COOKIE_PATH,
Trailing comma causes IE to get angry

> 
>+
>+TUI_alternates['expert_fields'] = 'Show Advanced Fields';
>+// Hide the Advanced Fields by default, unless the user has a cookie
>+// that specifies otherwise.
>+TUI_hide_default('expert_fields')
IE doesn't like the missing semicolon either

These both caused IE6 and IE7 to throw JS erros so they're more than nits. Plus i had to track them down b/c ie has such a great debugger!

Once these are fixed i can check more. I'll also check safari and chrome after IE passes.
Attachment #358435 - Flags: review?(guy.pyrzak) → review-
Attached patch v4Splinter Review
Okay, fixed the IE problems and put COOKIE_PATH into a global constant object called BUGZILLA. In the future we can also put constants in there, so that some code that's currently inside of templates can be moved into external files.
Attachment #358435 - Attachment is obsolete: true
Attachment #360826 - Flags: review?(guy.pyrzak)
Comment on attachment 360826 [details] [diff] [review]
v4

tested in IE and Firefox works in both without errors. Since I'm a half reviewer I'm going to pass it on to LpSolit.
Attachment #360826 - Flags: review?(guy.pyrzak)
Attachment #360826 - Flags: review?(LpSolit)
Attachment #360826 - Flags: review+
Comment on attachment 360826 [details] [diff] [review]
v4

Works fine on Fx 3.1b2. r=LpSolit
Attachment #360826 - Flags: review?(LpSolit) → review+
Flags: approval+
Whiteboard: [survey-important] → [survey-important] relnote
Keywords: relnote
Whiteboard: [survey-important] relnote → [survey-important]
Checking in js/TUI.js;
/cvsroot/mozilla/webtools/bugzilla/js/TUI.js,v  <--  TUI.js
new revision: 1.2; previous revision: 1.1
done
Checking in js/util.js;
/cvsroot/mozilla/webtools/bugzilla/js/util.js,v  <--  util.js
new revision: 1.8; previous revision: 1.7
done
RCS file: /cvsroot/mozilla/webtools/bugzilla/js/yui/cookie.js,v
done
Checking in js/yui/cookie.js;
/cvsroot/mozilla/webtools/bugzilla/js/yui/cookie.js,v  <--  cookie.js
initial revision: 1.1
done
Checking in skins/standard/global.css;
/cvsroot/mozilla/webtools/bugzilla/skins/standard/global.css,v  <--  global.css
new revision: 1.59; previous revision: 1.58
done
Checking in template/en/default/attachment/createformcontents.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/attachment/createformcontents.html.tmpl,v  <--  createformcontents.html.tmpl
new revision: 1.3; previous revision: 1.2
done
Checking in template/en/default/bug/create/create.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/bug/create/create.html.tmpl,v  <--  create.html.tmpl
new revision: 1.92; previous revision: 1.91
done
Checking in template/en/default/global/header.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/global/header.html.tmpl,v  <--  header.html.tmpl
new revision: 1.60; previous revision: 1.59
done
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Added to the release notes for Bugzilla 3.4 in bug 494037.
Keywords: relnote
You need to log in before you can comment on or make changes to this bug.