[0.7.5] Adding users to existing projects

[ambra]

I found a bug when trying to add a user to existing project. This is always reproducable.

Way to reproduce:

1. Login as administrator.
2. Go to Administration->Project administration, and the project list will be displayed.
3. Click on small triangle near the project title, and there will be a list of users displayed.
4. Click "Cancel".
5. After clicking on the triangle again, a new empty user will be added to the project.

You can add multiple empty users this way. These users will not be added to "users" table, but they will always be displayed here.

[Philipp]

Confirmed.
This is because the cancel button in that place seems to be submitting the form even if nothing is select - instead of canceling.

Will fix for next release.

[some person]

First problem here is that the admin is able to add a user, to the project, without actually selecting a user which they want to add. There should be a check, backend and front end, that checks if the user is submitting the form without having a user selected, and notifies the users.

The problem with the cancel button, in this instance, it works same as the submit button. The reason for this is that the cancel button has no button type declared in the source. Having no button type declared makes the browser treat at as a submit button. Next thing is the cancel button has some javascript, that is to take effect on clicking the button, but this javascript does not function properly, and this is why the button is handled as a submit as it has no action.

The proper fix here would be to add button type to the source. As this button has a on click, the button type would have to be button. So the source would look like something like the following

Code: Select all

<button type="button" onclick="javascript action">Cancel</button>

but as the on click javascript doesn't function, clicking it, does nothing.

The question now is what should the cancel button actually do. Should it reset the user selection, toggle/close project user list, or something else. Personally, in this instance, I do not see the point of having a cancel button. As the user, if they have changed their mind, and do not want to add a user, can void their actions just by not clicking on the add button.

Second instead of using a drop down list, I think it should be a user list instead, that allows multiple selections. This way the admin can add multiple users, all at once, without having to add each person separately.

Now on to fixing this. My recommendations would be to completely remove the cancel button, not only is it not necessary, but I think it can also confuse people. If you are wanting to remove the cancel button, remove the following line found in adminprojects.tpl

Code: Select all

<button onclick="blindtoggle('form_member');toggleClass('addmember','add-active','add');toggleClass('add_butn_member','butn_link_active','butn_link');toggleClass('sm_member','smooth','nosmooth');return false;" onfocus="this.blur();">{#cancel#}</button>

If you are not wanting to remove the button, and you are wanting it to reset the user selection and toggle/close the project user list, replace line 183, mentioned above, found in adminprojects.tpl, with the following.

Code: Select all

                                                   <button type="reset" onclick="javascript:accord_projects.activate($$('#acc-projects .accordion_toggle')[{$smarty.section.opro.index}]);toggleAccordeon('acc-projects',this);">{#cancel#}</button>

Now we can even go further, and make the user field required, and have it trigger the javascript validation. To do this, in adminprojects.tpl, replace lines 167 to 173 with the following.

Code: Select all

                                          <form  novalidate="novalidate" class = "main" method="post" action="manageproject.php?action=assign&amp;id={$opros[opro].ID}&redir=admin.php?action=projects&mode=useradded" {literal}onsubmit="return validateCompleteForm(this);"{/literal}>
                                             <fieldset>

                                                <div class="row">
                                                   <label for="addtheuser">{#user#}</label>
                                                   <select name="user" id="addtheuser" required="1" exclude="-1" realname="User">
                                                      <option value = "-1">{#chooseone#}</option>

The last thing to do, would be to add some PHP code to the backend to actually validate and check if the admin is submitting the form without a user selected. And also, one more thing I noticed, not allow the admin to add the same user multiple times.

[Philipp]

Some person:
I agree that the problem here is not limited to "cancel button actually submits the form" - i agree that to fix this properly a front and backend check is needed.
I just simplified in my reply, when giving the cause for this.

So what do we need the cancel button for?
a) consistency - all the forms where you can submit something have a cancel button - so this one should not be an exception
b) to slide the part of the UI back into closed state again when no action should be taken

[some person]

So what do we need the cancel button for?
a) consistency - all the forms where you can submit something have a cancel button - so this one should not be an exception

Consistency is good, but I think the cancel buttons are not necessary at all, and in some parts confuse people. So I do think it would be a good idea to remove them completely.

For example in some pages, inside collabtive, you can get to some of the pages through more then one path, but clicking on some of the cancel buttons, actually does not always get the user back to the previous page, where he/she was before, and I do think that the users expected outcome, when clicking on the cancel button, is to go to the previous page.

Cancel buttons are a good thing on things where action is needed, things like having inline popups, which has a form. In this instance the cancel button can be used to close the popup. Using it in this instance would not be confusing some people.

b) to slide the part of the UI back into closed state again when no action should be taken

Here I do not think that the user expects this outcome when clicking on cancel. If the cancel buttons are to be there, adding additional text, next to the cancel button, along the lines of "Cancel and close form" would be good idea. This would make sure that the user is not caught off guard, and is aware of what is to happen when clicking the cancel button.

[Eva]

Consistency is good, but I think the cancel buttons are not necessary at all, and in some parts confuse people. So I do think it would be a good idea to remove them completely.

Having a cancel option is a basic best practice regarding GUI's.

[ambra]

I've just noticed that it's also possible to add the same user twice.

Regards

[some person]

Consistency is good, but I think the cancel buttons are not necessary at all, and in some parts confuse people. So I do think it would be a good idea to remove them completely.

Having a cancel option is a basic best practice regarding GUI's.

I would agree with your statement, but only when the cancel button is used in the right places. I think, the way it's used in collabtive, has the potential to confuse people. I believe that seeing the cancel button makes the user think that it is required to click it, or if they do not, that their actions might not be voided.

Removing some of the cancel buttons, at least the ones that are on a page that only contain a form, would send a message, to the user, that they should, and could, possibly, use the browser's back button.

One problem with the cancel button is what should the action be. Most people would expect to be sent to the previous page they were on, but the problem with that is, which page would that be, as it is possible to get to some of the forms using more then one path.

I think that this whole thing could be made less confusing by using inline popups to display the forms, then in this case, using the cancel button, inside the inline popup, would be appropriate.

[Eva]

In Collabtive version 1.0 the cancel button will be gone.