179626 - Hide, rather than disable, requestee fields when the flags aren't in the "?" state

Status: RESOLVED FIXED

(Bugzilla :: Attachments & Requests, defect)

Description

[Boris Zbarsky [:bzbarsky]]

STEPS TO REPRODUCE:

1) Load "Edit an attachment" page
2) Request a review from someone (set the review flag to '?')
3) Submit form
4) Load the same page again
5) Set the review flag to '+'

ACTUAL RESULTS:  The "requestee" textfield becomes disabled but makes it look
                 like the review will be marked as being done by the requestee
EXPECTED RESULTS: The "requestee" textfield should be completely blanked out or
                  hidden to make it clear that it is irrelevant.

Comment 1

[Frédéric Buclin]

IMO, I would tend to mark it as WONTFIX. I doubt this really generates any confusion.

Comment 2

[Boris Zbarsky [:bzbarsky]]

Well, it sure confused me, and I'm not that easily confused.

Comment 4

[Max Kanat-Alexander]

Pyrzak's students' research pointed out that the grayed-out requestee box on
flags is confusing to new users--why is it there? Why can't they click in it?
The box should just be hidden when the flag isn't set to "?". This includes show_bug.cgi, enter_bug.cgi, and attachment.cgi.

Comment 5

[Reed Loden [:reed]]

Attachment: patch - v1

Comment 6

[Reed Loden [:reed]]

Attachment: patch - v1.1

Ok, this works. It uses parentNode to hide the entire <span> that contains the requestee field.

Comment 7

[Max Kanat-Alexander]

Comment on attachment 454445 [details] [diff] [review]
patch - v1.1

>+      requesteeField.parentNode.style.display = "block";

  Instead of manually setting the "display" attribute, you should add and remove the bz_default_hidden class, using YUI.

Comment 8

[Reed Loden [:reed]]

Attachment: patch - v2

Use addClass() and removeClass().

Comment 9

[Max Kanat-Alexander]

Comment on attachment 454750 [details] [diff] [review]
patch - v2

  This looks great!

>=== modified file 'js/flag.js'
>+YAHOO.util.Event.addListener(window, "load", hideRequesteeFields);

  On checkin, could you make that an onDOMReady? I'm getting a flash of unstyled content and I think perhaps onDOMReady will do it better.

Comment 10

[Reed Loden [:reed]]

Attachment: what I landed

Comment 11

[Reed Loden [:reed]]

Committing to: bzr+ssh://bzr.mozilla.org/bugzilla/trunk/
modified js/flag.js
Committed revision 7256.

Comment 12

[Max Kanat-Alexander]

Added to the release notes in bug 604256.