Closed Bug 373418 Opened 13 years ago Closed 5 months ago
Hide Bug Fields Editing Form By Default (show
I've decided to split up my work on bug 373105 into separate patches. This one is the basic one, that just hides the edit form for people who aren't looking for it (which is most users). I also changed the interface on TUI.js to be a bit more sensible. This is the first time we've actually used it in Bugzilla code, and there *is* a comment at the top that says the interface may change. So I've changed it.
Attachment #258079 - Flags: review?(LpSolit)
A URL where the patch is active is included in the URL field.
Comment on attachment 258079 [details] [diff] [review] v1 Tru, do you feel comfortable enough with JS to review this?
Attachment #258079 - Flags: review?(kevin.benton)
Attachment #258079 - Flags: review?(kevin.benton) → review+
Attachment #258079 - Flags: review?(LpSolit) → review?(reed)
Attachment #258079 - Flags: review?(reed) → review?(bugzilla-mozilla)
Attachment #258079 - Flags: review?(bugzilla-mozilla) → review?(LpSolit)
Comment on attachment 258079 [details] [diff] [review] v1 >Index: js/TUI.js > // Cookies are definitely disabled for JS. > if (TUI_getCookie(cookiesuffix).length == 0) >- TUICookiesEnabled = 0; >+ TUICookiesEnabled = true; > else >- TUICookiesEnabled = 1; >+ TUICookiesEnabled = false; > } These changes look wrong to me: 0 -> true and 1 -> false ??
Attachment #258079 - Flags: review?(LpSolit) → review-
I addressed all of LpSolit's comments, in one way or another. I have a lot of reviews for him in the queue, so Wurblzap--do you think you could get to this in the next few days?
The only thing I forgot to mention is: The <script> element is inline because that way it happens right after the appropriate section is rendered. If we make it an onload, it actually doesn't happen until the whole page is loaded, which sometimes is way too long. Also, I've had experiences with onloads generally not working well.
Priority: P1 → --
Oh, the previous version was missing show_bug.css.
Comment on attachment 274153 [details] [diff] [review] v2.1 I'm not a JS guy, so I will never r+ a patch which changes so much code in TUI.js. myk seems the right reviewer for this patch. That being said, here is what I saw so far: - Use relative URLs in show_bug, i.e. omit correct_urlbase. - You say you don't want the bug ID to be crossed out when the bug is resolved, but the alias is and it really doesn't look nice. - I personally don't want this table, at all. Having it displayed at the same time as the editable fields is something I cannot understand. Either add a user pref to let me hide your table in all cases, or hide your table when editable fields are displayed, and only redisplay your table when editable fields are hidden again. Else seeing it all the time will make me crazy very quickly. - Your table is broken when usetargetmilestone is off. Columns are shifted by one because you still try to display the target milestone despite the column header is gone.
Attachment #274153 - Flags: review?(LpSolit) → review-
Comment on attachment 281870 [details] [diff] [review] v3 Whoever gets to reviewing this first is fine with me.
Comment on attachment 281870 [details] [diff] [review] v3 This is pretty confusing to look at... The first time I visit a bug, I'm presented with both the small table and the edit fields, with the toggle in the middle between them. I click the edit/show link and the edit table disappears. huh? I think this should show one or the other not both, and the link to toggle should change to reflect which is currently showing. I'm at a tossup whether this is worth having a cookie remembering it... on the one hand, it's an extra step to edit. On the other hand, you're gong to wind up with the edit fields showing until you close it after you've edited one bug, and that might not be the desired behavior if you're just wanting to view the other bugs without editing them (since the short form is easy to read). I also wonder if we still want the help links even on the short version. Just because you're not editing the fields doesn't mean you're not going to want to know what they are.
(In reply to comment #12) > The first time I visit a bug, I'm presented with both the small table and the > edit fields, with the toggle in the middle between them. I click the edit/show > link and the edit table disappears. huh? You must have reviewed a previous version of the patch, or perhaps played with the mock-up in bug 373105. > I think this should show one or the other not both, and the link to toggle > should change to reflect which is currently showing. Yes, I certainly agree with that. I just thought that this was the simpler code change for now. > I'm at a tossup whether this is worth having a cookie remembering it... on the > one hand, it's an extra step to edit. On the other hand, you're gong to wind > up with the edit fields showing until you close it after you've edited one bug, > and that might not be the desired behavior if you're just wanting to view the > other bugs without editing them (since the short form is easy to read). Sure, but if you close it on the next bug, it will remain closed on all the others. > I also wonder if we still want the help links even on the short version. Just > because you're not editing the fields doesn't mean you're not going to want to > know what they are. Oh, yeah, we could do that. That's not too hard.
Okay, this one does what you want! (And what I wanted.) In order to do it reliably, I have to use the DOM. So now we only support IE6+ with this, and any other DOM1 browser. I made it so that if JS is off, or DOM1 isn't supported, everything is shown, so that things still work fine when we don't support DOM1.
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
Comment on attachment 281977 [details] [diff] [review] v4 Canceling this review request, as I'm no longer able to do Bugzilla reviews.
Comment on attachment 281977 [details] [diff] [review] v4 Severe bitrot
Attachment #281977 - Flags: review?(justdave) → review-
We are going to branch for Bugzilla 4.4 next week and this bug is either too invasive to be accepted for 4.4 at this point or shows no recent activity. The target milestone is reset and will be set again *only* when a patch is attached and approved. 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.
Target Milestone: Bugzilla 4.4 → ---
Status: NEW → RESOLVED
Closed: 5 months ago
Resolution: --- → FIXED
Target Milestone: --- → Bugzilla 6.0
You need to log in before you can comment on or make changes to this bug.