Issue266

Title Buttons to add/remove self from nosy list, assign issue to self
Priority feature Status resolved
Superseder Nosy List ajaksu2, amk, eric.araujo, ezio.melotti, loewis
Assigned To ezio.melotti Topics

Created on 2009-04-08.00:12:12 by ajaksu2, last changed 2010-07-07.07:36:35 by ezio.melotti.

Files
File name Uploaded Type Edit Remove
add_self3.diff ajaksu2, 2009-04-26.23:40:25 text/plain
issue266-+.png ezio.melotti, 2010-06-19.15:20:23 image/png
issue266-addme.png ezio.melotti, 2010-06-19.15:20:10 image/png
issue266.diff ezio.melotti, 2010-06-18.15:41:27 text/plain
issue266B.diff ezio.melotti, 2010-06-19.15:11:17 text/plain
issue266C.diff ezio.melotti, 2010-06-25.23:35:11 text/plain
Messages
msg1311 (view) Author: ajaksu2 Date: 2009-04-08.00:12:07
Makes assigning an issue to self and adding/removing self to/from the nosy list
easier. This patch adds two helper buttons that are hidden by default and set
visible by JS, so users with JS disabled won't see them.

Clicking on the 'Claim' button sets the user as assignee. The nosy button start
values is either 'Add myself' or 'Remove myself', according to whether the user
is already present in the nosy list. Clicking it adds or removes the user
to/from the list, toggling the button value.

My JS is even worse than my Python (even after JSLint), so any feedback is most
welcome.

Thanks Ezio Melotti for the nice RFE :)
msg1312 (view) Author: ajaksu2 Date: 2009-04-08.01:05:19
Some improvements in the JS code.
msg1363 (view) Author: loewis Date: 2009-04-25.09:21:19
What's the reason for putting the JavaScript into strings for tal:content,
instead of putting them into the content in the first place?
msg1375 (view) Author: ajaksu2 Date: 2009-04-26.23:40:25
I started with 'hardcoded' usernames in JS (set from TAL), and forgot to clean
this up when it became unnecessary. 

This new version removes the TAL setting of username on the buttons, doing it
once in a <input type="hidden"> and using JS to get the value.
msg1677 (view) Author: ezio.melotti Date: 2010-06-18.15:41:27
Here is a new patch to add the 'add me to nosy' button.

While doing the patch I took the following decisions:
1) Add self to nosy is a common operation and it's not easy to do (either click on the field and write the username, or click on "list", fill in the form, search, select the name, apply), so it definitely deserves a button;
2) On the other hand, removing self from nosy is not that common and it's easier to do (select the name and hit delete on the keyboard), so IMHO it doesn't deserve a button;
3) Assign an issue to self is not a common operation and it can be already done quite easily (click, scroll, click), so it doesn't deserve a 'claim' button;

In this way an 'Add me' button will be visible only on the issues where the user is not nosy. If the user decides to add himself to the nosy the button will disappear and the user won't see any other button while visiting the issue again the following times. If the user doesn't care about that issue it will just leave it there. Therefore the impact on the tracker should be minimal.

In the patch I used TAL to create the button only if the user is registered and the name is not in the nosy list. The button has display:none and it's made visible by javascript. This makes sure that if the user doesn't have JS enabled the button won't be available.
I put the JS code to make the button visible and the callback function to add the user to the nosy list in a separate file to keep the JS separate from the HTML.

The following thing should be checked before committing:
1) does the script deserves a whole new js file? if so a more appropriate name should be selected (I used help_controls2.js because there is already an help_controls.js file -- also note that I could use the same file for other patches, I just didn't use help_controls.js because I wasn't sure it was related in any way);
2) is the position and size of the button OK?;
3) when the button is pressed the name is added and then it disappears, causing a minor change in the layout. This should usually be avoided, but IMHO in this case is not a big problem, since I don't see how it can practically affect the usability.
msg1678 (view) Author: ezio.melotti Date: 2010-06-19.15:11:17
Here is another version (issue266B.diff) of the same patch that instead of adding an 'Add me' button under the input box, adds a [ + ] button on the right.
Even if less obvious than the 'Add me' button, it's less invasive, it doesn't take another line and the layout doesn't when the button disappears. To make it more obvious, the text 'Add me to the nosy list' appears when the pointer is on the button.
Another advantage is that some users might expect the 'Add me' button to submit the change immediately when it's still necessary to click on 'Submit changes'.
msg1681 (view) Author: eric.araujo Date: 2010-06-19.18:13:37
I don’t know TAL at all but is it normal that the variable in this line
  onclick="add_to_nosy(the_current_username)"
does not match the one in this previous line:
  tal:define="current_user request/user/username"

More important remark: What about people without JavaScript?
msg1682 (view) Author: ezio.melotti Date: 2010-06-19.18:25:20
'the_current_username' is just a placeholder and it is replaced by the tal:attributes in the previous line.
The button won't be displayed for people without Javascript because it's hidden by default using CSS.
msg1683 (view) Author: eric.araujo Date: 2010-06-19.19:44:47
I’m concerned that people without JavaScript won’t benefit from this improvement, and that people without JavaScript nor CSS will get the button but not the functionality.

One way to look at Web development is to write HTML+CSS+JavaScript and think about graceful degradation, whereas another way is to write HTM+server-side code that work, and then add CSS and JavaSript as progressive enhancement. It’s a bit more work, since you have e.g. to dynamically add controls that trigger JavaScript callbacks or modify existing controls. Roundup is near entirely usable (and I mean not only possible to use, but really fine) without CSS, JavaScript nor frames, and it’s a good thing™.

I won’t scream or ague to death if my concern is rejected. I wanted to express it, now that’s done, and it’s up to you to accept or reject it. Thanks for your work :)
msg1684 (view) Author: eric.araujo Date: 2010-06-19.19:49:15
s/ague/argue/
msg1685 (view) Author: loewis Date: 2010-06-19.20:44:11
I think I like the "Add me" version better; the single plus gives no clue what it will do.

As for JavaScript: I think it's fine to use it in this case. People without JS can still add themselves manually. If people are negatively affected by this change, they still can report that here (and failing any fix, we could revert the change).
msg1686 (view) Author: amk Date: 2010-06-20.16:36:42
I prefer 'Add me' instead of '+', but am worried that most people will not expect that submitting the form is necessary; most AJAX web sites will make the change immediately.   Maybe different text could clarify this, but I can't think of an alternative wording.

I suggest keeping all the JS code in one file (help_controls.js), so that there's one place to look for the definition of a particular function or handler.
msg1688 (view) Author: ezio.melotti Date: 2010-06-25.23:35:11
Here is a new patch (266C).

> I prefer 'Add me' instead of '+', but am worried that most people will not expect that submitting the form is necessary;
That's the reason why I don't like it. This new patch uses the [+] button (a few more developers on #python-dev prefer the [+]) and also has 'Add me to nosy (remember to Submit Changes)' as title="" text.

> I suggest keeping all the JS code in one file (help_controls.js), so that there's one place to look for the definition of a particular function or handler.
Since the other js file is not used by issue.item.html, I decided to put the js code in issue.item.js, also because the functions are specific to issue.item.html. This should make finding definitions even easier.

> I think I like the "Add me" version better; the single plus gives no clue what it will do.
I think that given the position, its purpose is quite obvious. The title="" text or even trying to press it to see what happens will clarify any doubt.

> [...] people without JavaScript nor CSS will get the button but not the functionality.
This is now fixed. The solution I adopted is to add an empty span that doesn't show up when CSS are not available, and then replace it with the button if JS is available. I tested it with a text browser (links) and it seems to work fine.
I agree with MvL that people without JS can add themselves manually. I also solved a couple of compatibility problems with IE.

The only remaining problem is that when a new issue is created the nosy list is empty and the button appears. I think it would be better if the user name would show up already when a new issue is created but I still have to figure out how to do it server-side.
Another minor improvement is to write the username always at the beginning of the list. In this way it's easier to select and delete self from the nosy even without a [-] button.

I suggest to commit the patch as is and see what people say.
msg1700 (view) Author: ezio.melotti Date: 2010-06-28.22:20:35
I committed the last patch in r82348.
I'm leaving this open waiting for some feedback.
msg1726 (view) Author: ezio.melotti Date: 2010-07-07.07:36:35
I did some minor tweaking in r82362 and r82412. This can be closed now.
History
Date User Action Args
2010-07-07 07:36:35ezio.melottisetstatus: in-progress -> resolved
messages: + msg1726
2010-06-28 22:20:35ezio.melottisetmessages: + msg1700
2010-06-25 23:35:12ezio.melottisetfiles: + issue266C.diff
messages: + msg1688
2010-06-20 16:36:43amksetmessages: + msg1686
2010-06-19 20:44:11loewissetmessages: + msg1685
2010-06-19 19:49:15eric.araujosetmessages: + msg1684
2010-06-19 19:44:48eric.araujosetmessages: + msg1683
2010-06-19 18:25:21ezio.melottisetmessages: + msg1682
2010-06-19 18:13:38eric.araujosetmessages: + msg1681
2010-06-19 15:20:23ezio.melottisetfiles: + issue266-+.png
2010-06-19 15:20:10ezio.melottisetfiles: + issue266-addme.png
2010-06-19 15:11:18ezio.melottisetfiles: + issue266B.diff
messages: + msg1678
2010-06-18 15:41:29ezio.melottisetstatus: chatting -> in-progress
nosy: + ezio.melotti, eric.araujo
messages: + msg1677
files: + issue266.diff
assignedto: ezio.melotti
2010-06-17 23:54:29amksetnosy: + amk
2009-04-26 23:40:26ajaksu2setfiles: + add_self3.diff
messages: + msg1375
2009-04-26 23:38:57ajaksu2setfiles: - add_self2.patch
2009-04-26 23:02:23ajaksu2setfiles: - add_self.diff
2009-04-25 09:21:20loewissetnosy: + loewis
messages: + msg1363
2009-04-08 01:05:19ajaksu2setfiles: + add_self2.patch
status: unread -> chatting
messages: + msg1312
2009-04-08 00:12:12ajaksu2create