Re-order the totals/moderating local tasks

Hey Berdir,

I like the new structure a lot. :-) Much simpler and cleaner! Here are a few things that I've noticed:

1. If you go to the "Users by Points" link (/userpoints), then a "(details)" link shows up next to your own username. But this link still takes the user to /myuserpoints and results in a Page Not Found error. In our new structure, isn't /myuserpoints no longer a valid link? If so, this details link should be pointing elsewhere.

2. When viewing another user's points transactions, it's a little weird that the breadcrumb shows "Home » My points". If you click on "My points" it takes you to your own points transactions page. So it sort of seems like the other user's points page is a subset of your own page. Should this breadcrumb be "Home >>Users by Points" instead?

3. Currently, when approving or declining points, I'm getting the following notice:

Notice: Undefined property: stdClass::$display in userpoints_tokens() (line 508 of /Users/benkaplan/git/drupal-7.0/sites/all/modules/userpoints/userpoints.module).

4. I like the per-transaction local tasks for "Approve" and "Decline", but it is slightly strange that you can go through the process of approving a transactions that has already been approved. If you click the "Approve" tab for an already approved transaction, it still asks you to confirm you want to approve it and then once approved gives you the drupal_set_message() confirmation... even though the transaction was already approved. Likewise, the same thing happens if you "decline" a transaction that was already declined. So do you think the "Approve" or "Decline" tabs should be hidden if it doesn't apply to the current transaction state? Or should there be some type of message that says "This transaction has already been approved" if you try to click the "Approve" tab when it is already approved (and vice versa)? Or maybe some type of CSS that greys out the state that it's already in and makes it unclickable?

5. It's now really easy for the approval/decline confirmation title to be hidden behind the four local task tabs. For instance, the title "Do you want to approve 3 kudos for Al Authenticated in the Goodwill category?" is only visible until the first half of the word "Goodwill". Is there any way to set the width of this or put it somewhere else so the full message is not obscured?

6. I like having +Add as a local task, but I'm not sure we need it on every single admin page. For instance, the +Add is showing if you're on the "Approve" or "Decline" tab for another transaction. In this case, it's just an unnecessary link that would probably never be clicked from that patch. So is there any way to hide it in a few instances like that?

7. When a user only has the "Add points" permission, should he be able to see the "Add or deduct points" link when visiting another user's profile page? Currently, such a user cannot see that link even though he has permission to add points.

8. If a user only has the "Edit points" permission, visits the sitewide points transactions page, and filters by category, then the following notice is displaying:

Notice: Undefined index: transactions in userpoints_admin_points() (line 409 of /Users/benkaplan/git/drupal-7.0/sites/all/modules/userpoints/userpoints.admin.inc).

--Ben

I tested the latest patch and it's starting to look real good. Consider an item fixed unless I mention it below:

5. I don't think shortening the title is going to be good enough. With those four big tabs in "Seven" there is hardly any room. Maybe we should put "Approve" or "Decline" (as the case may be) as the title. Then, in the white content area beneath , we can display the full question "Do you want to...?" in a bigger, bolder font than the other text. What do you think?

6. I agree that just "Add" is too short. I think "Add !points" is better, but I'm wondering if it should actually be "Add !points transaction" since that would only be for administrators. Also, at some point we've talked about allowing different branding for each points category. So not sure if we should make it more generic like "Add transaction". What do you think?

As for where to hide it, I would hide it from the following:

* the "!Points Settings" tab
* the "Approve" tab
* the "Decline tab

It's currently displayed on the view and edit pages of individual points transactions. I'm okay with this, but if clicked in that context it should really load the version of the "Add points" form with the username already filled in (the user whose points transaction you are viewing/editing). This would be like we do when the "Add or deduct points" link is clicked. Is this possible?

NEW STUFF:

9. I like the new second-level local tasks when viewing an individual transaction, but one thing is confusing... "List" takes the user back to the global transaction list page, but when you're viewing an individual user's transaction, you expect this to take you to the transactions list page for that individual user . Any way to change where this goes based on the context? I realize this may not be possible, but I thought I'd mention it.

10. When you are viewing the points transaction list page for an individual user, there is no easy way to get back to that user's main account page. Should we put a link at the top (the username/realname of the user as the link) that links back to the main account page. Or should the breadcrumb show the user's name so that it reads "Home > BenK"? I just think a common workflow for admin will be visit the user's account page, then view points transactions, then go back to user account page to do additional admin tasks.

11. When editing a pending transaction, all of the second-level local tasks are virtually running into the long breadcrumb (at least in the seven theme). One partial solution may be to shorten the local task labeled "View points transaction #414" . How about: "View transaction #414" Any other ideas? It's just that the breadcrumb is really long, too: "Home » Administration » Configuration » People » Points". But there may not be a way to shorten that. We could also possible get rid of "List" as a second-level local task since you can always click on the main "Transactions" tab to get back to the global transactions list.

--Ben

I just tested your latest patch and it's looking great! Here are my comments:

5. I like how this looks a lot. Totally solves the width issue for me. I've attached a screenshot so that you can see. Possibly one suggestion: Instead of having "Approve" and "Decline" as the titles of each respective overlay, would you prefer "Approve transaction" and "Decline transaction"?

6. This looks great! :-)

9. Actually, what I meant is that in the current implementation "List" doesn't always get you back to the List where you came from. It works fine if you came from the Userpoints admin back office. But if you were viewing a user's points transaction page, then click "edit" (to edit a user's transaction), then click "List", you are taken to the global list page instead of the user's list page. Not a huge deal, but I hope this is more clear. Basically, there are two possible paths to get to a user's edit transaction page and the "List" tab only takes into account one possible path.

10. How about this simple solution: When visiting a user's points transaction list, the title of the page is: "!Points for BenK". Can we make the username (BenK) a link back to the user's main account page? We do that with most other instances in which a username is mentioned for PM, UP, and UR.

11. The width isn't a problem for me anymore. Not sure why it was. However, I still think that we should probably shorten the local task labeled "View points transaction #414" . I'd recommend "View transaction #414". I just think that four words is pretty long for one of those sub-tabs (much longer than all the other ones). It would look better a bit shorter, I think.

When actually viewing the transaction, it is also debatable whether the page title should be "View !points transaction #414" or just "View transaction #414". I'd tend to be kinda wordy in the interface strings, so I've been trying to fight that urge. Thoughts? I'm probably fine either way.

Probably one more pass and this is ready to commit! :-)

--Ben

Nice suggestions in comment #8... I agree with all of them! ;-)

--Ben

Item 11) looks nice now. :-)

But on 5), I think you may have misunderstood. I think the sub-local task tabs should still be "Approve" and "Decline" (this is the part you changed). But instead, I think the page title for the overlay should be "Approve transaction" and "Decline transaction" (this part wasn't changed). After that fix, this should be good to go.

--Ben

P.S. I'll take a look at the userpoints_contrib stuff...