This patch makes a few changes to the message list:

  • Changed the names of the all but one of the columns, making them more in line with what you'd see in an e-mail client. This is probably better done by changing the database fields names, but I'll leave that to the maintainer.
  • Unread (not "new"!) messages are displayed in bold.
  • The timestamp ("sent" date) is formatted according to the site's short date setting.
  • An Unread indicator is included. I'm not happy with this, but it beats a '1' or a '0'. I'd be okay with dropping this column completely and letting the bold text indicate unread messages, but an image in this column would look good and still allow the user to sort by Unread.
  • The 'From' column for Sent messages and 'To' column for the Inbox only show if the user is viewing another user's messages (with 'read all private messages' permission, of course).
  • The author and recipient of a message, when shown, are themed. (nbz)
  • The remaining database fields/columns are removed. (nbz)
CommentFileSizeAuthor
#18 privatemsg_list.patch6.39 KBnaheemsays
#16 privatemsg_message_list-298492-16.patch4.98 KBmrtoner
#11 privatemsg_message_list-298492-11.patch3.75 KBmrtoner
#11 privatemsg-list.css_.txt67 bytesmrtoner
#10 message_list_alt.patch3.41 KBnaheemsays
#9 message_list_alt.patch3.34 KBnaheemsays
privatemsg_message_list.patch3.28 KBmrtoner

Comments

naheemsays’s picture

naheemsays commented

If the unread messages are in bold, we probably do not need another column to indicate unread status.

If extra visibility is needed, maybe a notice after the message subject, something like <unread>, but using a column for this data seems wasteful to me.

Apart from that nitpick, I like.

litwol’s picture

litwol commented

Is this patch against HEAD or is it on top of http://drupal.org/node/291179 ? It applied against HEAD without a problem but i am getting a ton of PHP notices.

here are a few:
When viewing a message:
notice: Undefined variable: message_subject in /www/drupalgames.com/sites/all/modules/privatemsg/privatemsg-view.tpl.php on line 14.

When inside sent messages folder:
notice: Undefined index: new in /www/drupalgames.com/sites/all/modules/privatemsg/privatemsg.module on line 294.
notice: Undefined index: author in /www/drupalgames.com/sites/all/modules/privatemsg/privatemsg.module on line 295.

When inside inbox:
notice: Undefined index: recipient in /www/drupalgames.com/sites/all/modules/privatemsg/privatemsg.module on line 298.

Lets remove the Unread column and instead _force_ all unread messages to be on top by default (without an option to sort by read/unread). lets fix those minor issues and this patch will go in! i really like this.

litwol’s picture

litwol commented
Status: Needs review » Needs work

forgot to set another status.

naheemsays’s picture

naheemsays commented

Is this with some sort of error reporting turned on? (if so, how?)

I do not get any errors

litwol’s picture

litwol commented

go to admin/settings/error-reporting and switch "Error reporting:" to "Write errors to the log and to the screen". This should do the trick. let me know if it doesn't and i'll help you with further configurations.

naheemsays’s picture

naheemsays commented

I already have that set.

litwol’s picture

litwol commented

In your settings.php, on the very first line inside and tags place the following code:

 error_reporting(E_ALL);
 ini_set("display_errors", 1);
litwol’s picture

litwol commented

nbz come chat with me on irc at http://nycdrupal.org/content/irc-drupalnyc-public-channel
i'll walk you through setting this up.

naheemsays’s picture

naheemsays commented
StatusFileSize
new message_list_alt.patch3.34 KB

Not fixed the error messages as I cannot reproduce them, but here is a version of the patch without a column indicating read status (leaving just the bold text to indicate that).

I would want to go further and only bold the subject, with maybe text saying unread after that.

I have also changed the <b> to <strong>.

naheemsays’s picture

naheemsays commented
Status: Needs work » Needs review
StatusFileSize
new message_list_alt.patch3.41 KB

ok, had a look at litwol's changes to my dev tracker patch and I understand what needs changing.

This patch should fix the notices.

mrtoner’s picture

mrtoner commented
StatusFileSize
new privatemsg-list.css_.txt67 bytes
new privatemsg_message_list-298492-11.patch3.75 KB

Hey, litwol, thanks for weighing in! The patch applies against head, not against #291179: My changes to privatemsg (dev tracker). I'm guessing you have changed your privatemsg-view.tpl.php, because HEAD doesn't reference that variable.

I'm attaching a revised patch that updates the "sent" column name (shows "received" in the Inbox, "sent" in Sent messages) and removes the Unread column, while forcing the default sort by Unread. It also incorporates nbz's latest changes to eliminate the "undefined index" errors. However, instead of using "<strong>" to indicate unread messages, I've chosen to use CSS classes and let the site developer style that as he likes.

In fact, to accommodate nbz's desire for just the subject being bolded on unread messages and mine for the entire line, I've included multiple classes -- and have also attached a sample stylesheet (with nbz's preference being the default).

litwol’s picture

litwol commented

wonderful! i'll get to reviewing when i get home.

naheemsays’s picture

naheemsays commented

When all the messages have been read, the sort order seems to be oldest first.

Other than that, :D

EDIT - the use of arg(1) would clash with #298502: Better access to another user's messages (if we go down that route). This clash may be easier to deal with in another issue after a few patches are committed/rejected, to see where things lie.

mrtoner’s picture

mrtoner commented

I just looked at the code and it will be trivial to fix both once we have a new HEAD.

naheemsays’s picture

naheemsays commented
Status: Needs review » Needs work

Addressing comment 13:

1. The order has been dealt with in #299491: Put a reply form on the message page
2. Dealing with incompatibility with [298502] should probably be left to that patch, leaving the reroll to either just match current HEAD, or HEAD + patch in point 1.

mrtoner’s picture

mrtoner commented
Status: Needs work » Needs review
StatusFileSize
new privatemsg_message_list-298492-16.patch4.98 KB

This patch has been re-rolled against HEAD and incorporates these additional changes:

1. Default sort order using nbz's code in #299491: Put a reply form on the message page to fix #13-1
2. Clicking column headers to sort wasn't working before, now it is

naheemsays’s picture

naheemsays commented

rerolled the above patch to go in after #297419: Cleanup comments & formatting - also included new css file in patch and removed other commented out code.

EDIT ignore this.

naheemsays’s picture

naheemsays commented
StatusFileSize
new privatemsg_list.patch6.39 KB

this should work (unless it clashes with more code cleanups)

EDI - the css is in comment 11. I expected diff to actually add the whole file, but it did not. Just a mention that it only exists in one, not the other.

litwol’s picture

litwol commented
Status: Needs review » Fixed

Patch commited: http://drupal.org/cvs?commit=137678

Few notes:

I'm opening 1 new issues:
1) use proper table sorting (http://api.drupal.org/api/search/6/tablesort) inside privatemsg_privatemsg_list_sent_alter and privatemsg_privatemsg_list_alter instead of current ordering method.

great job guys. thank you.

Anonymous’s picture

Anonymous (not verified) commented
Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for two weeks with no activity.