This module adds a file field formatter to display molecular structure files using the Jmol java applet for molecular biology scientists. The applet code (library) itself has to be downloaded separately.
I have met many bioinformatics scientists and biology teachers who want to embed the Jmol molecular viewer in their Drupal installation, but just can't get it to work. A detailed README.txt file helps with the installation of the module and an example file in the docs subfolder helps users to write their own pages with customized applets.
To my knowledge, there are no similar projects.

Link to project page: http://drupal.org/sandbox/jvdurme/1766006
git clone http://git.drupal.org/sandbox/jvdurme/1766006.git jmol
Drupal version 7
Demo: http://jmoldemo.switchlab.org/node/1

cheers,

Joost Van Durme

Reviews of other projects
http://drupal.org/node/1804464#comment-6581998
http://drupal.org/node/1777990#comment-6458518
http://drupal.org/node/1777990#comment-6476594
http://drupal.org/node/1777334#comment-6471700 (reported duplicate, saves us all some time)
http://drupal.org/node/1778036#comment-6451622

More reviews
http://drupal.org/node/1777990#comment-6583070
http://drupal.org/node/1810360#comment-6592296
http://drupal.org/node/1597944#comment-6592368
http://drupal.org/node/1800900#comment-6598700
http://drupal.org/node/1553456#comment-6612918
http://drupal.org/node/1812464#comment-6622424

Comments

ankitchauhan’s picture

welcome,

As installation and usage instructions are quite important for us to review, please take a moment to make your project page follow the tips for a great project page. Also make sure your README.txt follows the guidelines for in-project documentation.

while waiting for an in-depht review of your module you can start out fixing some coding style issues detected by automated tools:
http://ventral.org/pareview/httpgitdrupalorgsandboxjvdurme1766006git

There is still a master branch, make sure to set the correct default branch: http://drupal.org/node/1659588 . Then remove the master branch, see also step 6 and 7 in http://drupal.org/node/1127732
The following git branches do not match the release branch pattern, you should remove/rename them. See http://drupal.org/node/1015226
remotes/origin/7.x-1.0

We do really need more hands in the application queue and highly recommend to get a review bonus so we can come back to your application sooner.

regards

jvdurme’s picture

Hi there,

thanks for the additional tips.

The master branch and the 7.x-1.0 branch have been removed from git.
All files were updated to have no warnings or errors from CodeSniffer.
The project page has been updated to be more informative and it looks better as well now.
I'll try to get that review bonus. :-)

cheers,

Joost

dharam1987’s picture

Hi,

I guess you might have missed this, a very small whilespace related error found during automated review. But this can lead to needs-work by admins.

Here is the error info

FILE: ...w/sites/all/modules/pareview_temp/test_candidate/docs/jmol.examples.php
--------------------------------------------------------------------------------
FOUND 1 ERROR(S) AFFECTING 1 LINE(S)
--------------------------------------------------------------------------------
56 | ERROR | Whitespace found at end of line
--------------------------------------------------------------------------------

jvdurme’s picture

Ha, indeed, a new whitespace crawled into that file. Thanks! Fixed.

jvdurme’s picture

Issue summary: View changes

Added demo site url.

jvdurme’s picture

Priority: Normal » Major

Changed status to major since 2 weeks without reviewer comments.

DmitriyMakeev’s picture

The result are a fairly simple module, which could implement with the page theming. It would be nice to add a formatter to display files. Than users can add new nodes with uploaded .pdb-file, to see new molecular structure. See Field Formatter API:
- hook_field_formatter_info
- hook_field_formatter_settings_form
- hook_field_formatter_settings_summary
- hook_field_formatter_view
where 'field types' => array('file').
With hook_field_formatter_settings_form you can change width, height and default style of the applet.

Besides you need to import library with Libraries module, and place library to "sites\all\libraries". It will make installation simpler, and you don't need to use $_jmol_folder.

DmitriyMakeev’s picture

Status: Needs review » Needs work
jvdurme’s picture

Priority: Major » Normal

So I have some challenges to tackle. ;)
Thanks for the tips, Dmitriy. I'll see how fast I am with formatters and try to implement Libraries.
If I can avoid changing the module code, that would be great indeed.

About your suggestion to use formatters. Is this intended to let users create their own jmol applet pages using a new content type? Or what would be your intention to use formatters?

Question is: do I stick with the idea that users have to change the module code to make jmol nodes or do I make a module that creates a new jmol content type so users can create jmol pages with custom pdb files and that nodes are being created for them? This would need a whole other take, but it's worth it I think. And more professional and less chance of breaking stuff.

DmitriyMakeev’s picture

I think there is no reason to create some new content type, only field formatter, like image or audio field formatters (Shadowbox or AudioField for example).

jvdurme’s picture

Yeah exactly. I have the libraries working already. I'll see for the formatters now.
Thanks for the tips!

jvdurme’s picture

Almost there with the file formatter...

Just curious.. is it possible to attach extra fields to the File field? Like description, prefix html, suffix html, ... specific for each uploaded .pdb structure file?
Then users can create whole pages with mixed text and applets. Like:

<Some text explaing the next molecule in Applet1>

<Applet1>

<Some text explaing the next molecule in Applet 2>

<Applet2>

Thanks!

jvdurme’s picture

Status: Needs work » Needs review

Hi all,

the module is now a file field formatter for molecular structure files.
For testing, the module folder includes such a file (1crn.pdb).

Thanks for reviewing!

Joost

jvdurme’s picture

Issue summary: View changes

Changed link to demo site.

jvdurme’s picture

Issue summary: View changes

Rewrote the entire module to be a file field formatter now.

jvdurme’s picture

Issue summary: View changes

Changed demo url.

jvdurme’s picture

Issue tags: +PAreview: review bonus

Applying for review bonus. :)

klausi’s picture

Status: Needs review » Reviewed & tested by the community
Issue tags: -PAreview: review bonus

Thank you for your reviews. When finishing your review comment also set the issue status either to "needs work" (you found some problems with the project) or "reviewed & tested by the community" (you found no major flaws).

manual review:

  1. jmol_enable(): why do you need that function?
  2. jmol_build_html(): you should really use Drupal.settings in javascript to pass variables from PHP to JS. See http://drupal.org/node/756722 . And the rest should go into a dedicated JS file added with drupal_add_js(). Same for jmol_page().
  3. jmol.examples.php: maybe that should go into a jmol_exmaples.module? So you could provide pages that users can actually try out.

Although you should definitely fix those issues they are not hard blockers, so I guess this is RTBC. Removing review bonus tag, you can add it again if you have done another 3 reviews of other projects.

klausi’s picture

Issue summary: View changes

Added reviews of other projects

jvdurme’s picture

Issue summary: View changes

updated reviews

jvdurme’s picture

Issue summary: View changes

added review

jvdurme’s picture

Hi Klausi,

thanks for the in depth review.
I've been experimenting with Drupal.settings a little, but there is a problem here.
In my jmol_build_html() function I'm building gradually the html output for the formatter. And the data needs to be in a specific order:

1. div container for applet alignment
2. display filename if set in the formatter settings
3. javascript section to actually display the applet on this spot
4. a textfield underneath the applet
5. closing div container

So if I would use drupal_add_js to add the javascript in point 3, there is no way I can force it to be inserted on this particular place in the html code. And it needs to be literally written there.

Do you know what I mean? Am I not seeing something perhaps?

cheers,

Joost

jvdurme’s picture

Issue summary: View changes

added review

jvdurme’s picture

Issue tags: +PAreview: review bonus

And back.
I have solved points 1 and 3.

1. I changed this to jmol_install() because it's not meant to be run at each enable. As the target audience might include lots of non-developer users, I want to display a message when the libraries are not correctly installed. I want to give them as much information as possible.

3. Yep, you're right. Would be a waste of code otherwise. This is now the "Jmol example pages" module that creates 3 pages with applets. I have commented the code enough to get starting coders on their way.

About point 2 I have to redirect you to post #15 of myself. There I explain why I can't use drupal_add_js() for the applet code, unless I'm wrong.

Thanks mate!

jvdurme’s picture

Issue summary: View changes

Added review.

jvdurme’s picture

Issue summary: View changes

Added review.

klausi’s picture

Status: Reviewed & tested by the community » Fixed

no objections for more than a week, so ...

Thanks for your contribution, jvdurme!

I updated your account to let you promote this to a full project and also create new projects as either a sandbox or a "full" project.

Here are some recommended readings to help with excellent maintainership:

You can find lots more contributors chatting on IRC in #drupal-contribute. So, come hang out and get involved!

Thanks, also, for your patience with the review process. Anyone is welcome to participate in the review process. Please consider reviewing other projects that are pending review. I encourage you to learn more about that process and join the group of reviewers.

Thanks to the dedicated reviewer(s) as well.

jvdurme’s picture

Wonderful klausi!
I have learned a lot while reviewing other projects and while reading other reviewers comments.
I will definately stay on this forum here to contribute reviews and to learn new stuff.
Thanks for the approval of Jmol!

Edit: I would like to thank all reviewers of Jmol. Especially DmitriyMakeev for turning a first try into a genuine module. Thanks all!

Joost

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

Anonymous’s picture

Issue summary: View changes

Added review.