[D7] Gallery 3D

Hi,

First of all I would like to say that this module looks nice. It does some cool gallery stuff and is easy to use.

Coding standards were applied successfully but I have just one remark.

Is there a configuration form to edit the speed of the gallery ?

It would be a really nice feature and maybe necessary.

Kind regards
Sander!

Hello itsmebhupendra and welcome!

/**
 * Implements hook_preprocess_node().
 */
function gallery3d_preprocess_node(&$vars) {
  if ($vars['type'] == 'gallery_3d') {
    drupal_add_js(drupal_get_path('module', 'gallery3d') . '/js/jquery.cloud9carousel.js');
    drupal_add_js(drupal_get_path('module', 'gallery3d') . '/js/jquery.reflection.js');
    drupal_add_js(drupal_get_path('module', 'gallery3d') . '/js/gallery_3d.js');
    drupal_add_css(drupal_get_path('module', 'gallery3d') . '/css/gallery3d.css');
  }
}

In general 3rd party libraries are forbidden.
Please see https://drupal.org/node/422996

Some jquery-plugins are provided by jQuery plugins. But unfortunately there is no jquery.cloud9carousel.js and jquery.reflection.js.

Hi itsmebhupendra,

I still see the Javascript code written in "node--gallery_3d.tpl.php" file (please see https://drupal.org/comment/8575877#comment-8575877).
I also suggest you to follow best practices of writing JS code (none of the JS statements have EOL).

Otherwise looks a very clean module to me!!

Regards,
Madhusudan

This module works fine and very easy to use as we don't need to do much effort to use this.
But there should be permissions and configuration page for the same so that we can manage its visibility for users with different permissions and animation speed, stop auto play etc.

Found the module very easy to install and use.

Could not spot any immediate errors with the code however after using the module and then disabling, uninstalling and then enabling again I got the error:

PDOException: SQLSTATE[23000]: Integrity constraint violation: 1062 Duplicate entry 'Gallery3D' for key 'name': INSERT INTO {image_styles} (name, label) VALUES (:db_insert_placeholder_0, :db_insert_placeholder_1); Array ( [:db_insert_placeholder_0] => Gallery3D [:db_insert_placeholder_1] => Gallery3D ) in drupal_write_record() (line 7194 of /var/www/test/htdocs/includes/common.inc).

So sounds like you need to make sure you are removing your image styles when uninstalling the module.

OK, @itsmebhupendra , I think there is a problem here.

Correct me if I'm wrong, but it sure looks a lot like your copy of jquery.cloud9carousel.js is just a copy of jquery.cloud9carousel.js (or a similar clone) that you removed the licensing information from the top of.
As Open-source workers here and in the Drupal project, it is of primary importance that users of the Drupal code repositories understand and respect the legal issue of software licensing and re-use, as well as just following the conditions for proper use of the Drupal repositories.

PAReview: 3rd party code

3rd party code is not generally allowed on Drupal.org and should be deleted. This policy is described in the getting involved handbook. It also appears in the terms and conditions you agreed to when you signed up for Git access, which you may want to re-read, to be sure you're not violating other terms.

The Libraries API module is a recommended method for adding 3rd party dependencies without directly including the code on Drupal.org.

I've not looked at the other code yet, but I would question the provenance of the other unattributed library you have in here also.

There are some errors reported by automated review tools, did you already check them? See http://pareview.sh/pareview/httpgitdrupalorgsandboxitsmebhupendra2214701git

I'm a robot and this is an automated message from Project Applications Scraper.

I got this error on admin/modules/list/confirm page:

FieldException: Attempt to create a field of unknown type <em class="placeholder">image</em>. in field_create_field() (line 110 of (...)\modules\field\field.crud.inc).
The website encountered an unexpected error. Please try again later. 

When I try to uninstall it, I got:
Fatal error: Call to undefined function image_style_delete() in (...)\sites\all\modules\custom\gallery_3d\gallery3d.install on line 144

It looks like, there are some missing dependencies. I recomend you add this line
dependencies[] = image
in to gallery3d.info file.

File sites\all\modules\custom\gallery_3d\js\jquery.reflection.js looks like Reflection.js plugin for jQuery (3rd party SW released under MIT License. http://www.digitalia.be/software/reflectionjs-for-jquery). I am not a licence expert, but there is removed licence info in your file. Also, better place for 3d party libraries is sites\all\libraries folder.

Gallery 3D is very nice, but a lot of "Requires" and it does not have a block, so there are difficulties when bringing it to the position that user wants. It would be great to make block for more convenient use of gallery

Giving it a run-through.

* Checkout out onto a mostly-clean site (standard theme + display suite) with demo content.
* Enabled OK.
* Not quite sure why it's providing a new content type, but OK.
* as there seems to have been no checks or warnings, I have a look at the content type and go straight to /node/add/gallery-3d
* adding images and things seems normal.
* viewing it however is pretty ugly. I guess it;s time to go and find the dependencies that it did not check for. I'd say you could probably double-check that everything was working on the node-edit form for thos people who don't read instructions. Not a blocker though.
* I know that when things are missing in Drupal, the place to go and find out about what I should do next is admin/reports/status ... but there seems to be no problems there!? - suggest you add a hook_requirements to help figure this out.
* I guess I'll go read the project page again. Following the install instructions you have there...
* your instructions say to copy the "cloud9carousel" library and place it in a folder called "sites/all/libraries/gallery3d/" this is the wrong way to name included libraries. We use the 'libraries' folder so that each module does not have to pull its own copy of a library that *may* be used by others. If you are using the 3rd party project called "cloud9carousel" then download it into "sites/all/libraries/cloud9carousel/" . It's not fair for you to rename someone else's work to your own.
* So I got the library into the right place. Refreshed my new album.
* still not looking right. I'm Using Chrome on OSX today. is there a browser problem?
* Opening up the console I see the javascript error
"Uncaught TypeError: undefined is not a function gallery_3d.js:16"
so ... hm.

Debugging, it looks like '#showcase' was not found on the page. Let's see the code then...
OK, it's a bad start with the jquery.
You must not use $(document).ready(
To avoid conflicts with all the other Drupal scripts, and AJAX and things, you should use

Drupal.behaviors.myModule = {
  attach: function (context, settings) { 
  }
}

This and more is documented in Working with Javascript and in examples you will find in Drupal already.
You MUST do this, or you will be looking to trigger errors in other folks code.

It doesn't look like that was what was causing my problem today - a cache-clear seems to have helped me.
BUT for code standards review, you must change the $(document).ready into a Drupal.behaviors attach function.

Looking at the code, it's not showing a lot of awareness for how Drupal actually works.
Providing a new content type and the node--gallery-3d.tpl.php to render it in full is not a good approach. Now nobody can add fields or any other Drupal additions to this content, or even choose to place the description text above or below. Using this with their own theme will also require extra theme code that could otherwise be avoided.
What you should do to incorporate this library is make the carousel display a field formatter that presents the attached images! This is much more flexible.
What you have here is very limited for re-use
If I found someone in my team building a module in this way, I would tell them to re-think it and rewrite it to be re-usable.

Other like
https://www.drupal.org/project/field_slideshow
https://www.drupal.org/project/galleryformatter
https://www.drupal.org/project/total_gallery_formatter
https://www.drupal.org/project/slick_slideshow
(and many many more)
All do it like that, as it's the most useful for other folk to work with.

I also see that there are also existing modules that provides integration with derivatives of the same library:
https://www.drupal.org/project/viewscarousel3d ( Professor Cloud Carousel )
https://www.drupal.org/sandbox/jamesbenison/1076640 ( Views jQFX Cloud Carousel )
Though they do seem to focus on Views and not be strongly active.

I say that the javascript and the hook_implements are required fixes. So needs work there.

I don't know about the rest of the code though. I feel it's low-quality due to the way it approaches the task, and needs an actual re-work to fit into the Drupal way. .. but others may have different opinions.

Closing due to lack of activity. If you are still working on this application, you should fix all known problems and then set the status to "Needs review". (See also the project application workflow).

I'm a robot and this is an automated message from Project Applications Scraper.