Chat with Themes, RumbleTalk

You got some formatting issues (See http://ventral.org/pareview/httpgitdrupalorgsandboxrumbletalk1369470git, you can use this site to re-test your self).

If you got questions on this, please ask!

just try it your self
go to http://ventral.org/pareview/httpgitdrupalorgsandboxrumbletalk1369470git
-> repeat review
and it will be scanned again, if there are still issues, you can use this to check your self while developing

Still issues of comment #1.

Not fixed for weeks -> needs work.

Please fix these issues, retest your self and change back to needs review.

Line 27, chat_with_themes__rumbletalk.module:

case "admin/help#onthisdate":

You should replace the path on this line with the path for your module's configuration page, so that the output is displayed on this page. ( replace the path with admin/config/user-interface/rumbletalkchat ).

Line 89, chat_with_themes__rumbletalk.module:

'#description' => t('This is the 8 letters/signs chat room code you\'ve received from RumbleTalk.<br/>If you do not have a chat room code, please register at <a href="http://www.rumbletalk.com/" target="_blank">RumbleTalk.com</a>, it\'s super fast and free.'),

You should encode the link wrapped in the t() using l(), and use a placeholder to include the link in the output.

The README.txt file asks the user to visit rumbletalk.com for more information about how to use the module, but I wasn't able to find the instructions anywhere on the site. I would add some instructions in the README and in the _help hook. There is no mention anywhere about how the chat widget is actually made to appear anywhere. By looking at the code, I can determine that the widget can be added by enabling a filter on one of my "text formats", but another drupal user who just installed your module would have no way of knowing this, or what filter token to use. In other words, the help and README should include something like:

- enable module
- go to admin/config/content/formats
- select the format you want to enable RumbleTalk on
- mark the checkbox for RumbleTalk chat
- when creating content, insert [rumbletalkchat] into the post where you want the widget to appear

Automatic review

There's seem to be some little things pending with the coding standards.
You can see them here:
http://ventral.org/pareview/httpgitdrupalorgsandboxrumbletalk1369470git-...

Manual review

README.txt

First thing I notice, like exposed by #7, is the lack of information in the README.txt file:
You don't talk about the configuration menu;
You didn't notice about the specifications of the text formats to choose (Plain text dosen't work for example).

Comments in code

It could be great to specify the hooks you implements like this:

/**
 * Implements hook_menu().
 *
 * @return array
 *   An array with this module's settings.
 */
function chat_with_themes__rumbletalk_menu( ) {
  ...
}

It's really more convenient for people who reads your code.

Module list

Suggestion: You could add the link to access the configuration panel in the module list (/admin/modules).
Instructions: http://www.drupalcoder.com/blog/configure-links-on-modules-overview-page...

Internalization

Some strings use the t() function, some no:

  ...
  $output .= 'You are not authorized to view RumbleTalk Chat, please configure access permissions.';
  ...
  $output .= 'Empty RumbleTalk Chat Code, please configure RumbleTalk Chat plug-in.';
  ...

Errors messages relevance

If I'm a simple visitor, this message will not really help me cause I can't configure access permissions or RumbleTalk plugin. The administrative part of the messages could be log with the watchdog() function.

You are not authorized to view RumbleTalk Chat, please configure access permissions.

Empty RumbleTalk Chat Code, please configure RumbleTalk Chat plug-in.

I've also notice that after I've refresh a test page multiple time, the plugin dosen't load. It's stock on loading. Maybe a timeout/access error should be add.

Suggestion: Given that the visitors can see these errors messages, maybe they should be configurable.

Form validation

I can write potatoe in the configuration form (/admin/config/user-interface/rumbletalkchat) and save without a problem.

Changing it to needs work based on #comment9.

Closing due to lack of activity. Feel free to reopen if you are still working on this application.

Image removal