Flipbook - View nodes as flip type [#1678784]

Flipbook module show nodes as flip style. Flipbook can only use in block, and provider some simple settings like control bar, enable default audio on flipping (only support on html5 browsers), width&height.

project link

git clone --recursive --branch master wayn@git.drupal.org:sandbox/wayn/1612282.git

code write for drupal7

Comments

Comment #1

FranciscoLuz commented 11 July 2012 at 05:16

Status:

Needs review

» Needs work

Hi,
Cool module.

Here are some observations you might consider working at before further do:

You should create a branch called 7.x-1.x and put your code there.
You should keep master branch empty.
Why so many underscores ( _ ) in the module's name?
I would suggest you calling it flipbook_content_type
What makes this module different from http://drupal.org/project/pageflip?
You should fix the errors listed at http://ventral.org/pareview/httpgitdrupalorgsandboxwayn1612282git
Note that some of the warnings/errors might be false positives and you are not required to fix all of them.

Comment #2

wayn commented 9 October 2012 at 06:40

Hi,
Sorry for reply so later, and thank you for your work.

I have done the check list you give me, my module do flip as book in a page this is the difference of that pagefilp module, you can see the demo for my module http://demo.webonn.com/drupal7/node/2

Comment #3

wayn commented 12 October 2012 at 02:45

Status:

Needs work

» Needs review

Hi,
Sorry for reply so later, and thank you for your work.

I have done the check list you give me, my module do flip as book in a page this is the difference of that pagefilp module, you can see the demo for my module http://demo.webonn.com/drupal7/node/2

git clone --recursive --branch 7.x-1.x wayn@git.drupal.org:sandbox/wayn/1612282.git

Comment #4

cubeinspire commented 13 October 2012 at 12:24

Status:

Needs review

» Needs work

Hi wayn !

1- I see there is a problem on uninstall:

    Warning: Illegal string offset 'field_name' in field_delete_instance() (line 740 of /var/www/d7/modules/field/field.crud.inc).
    Warning: Illegal string offset 'entity_type' in field_delete_instance() (line 741 of /var/www/d7/modules/field/field.crud.inc).
    Warning: Illegal string offset 'bundle' in field_delete_instance() (line 742 of /var/www/d7/modules/field/field.crud.inc).
    Warning: Illegal string offset 'field_name' in field_delete_instance() (line 749 of /var/www/d7/modules/field/field.crud.inc).
    Warning: Illegal string offset 'field_name' in image_field_delete_instance() (line 471 of /var/www/d7/modules/image/image.module).

You are missing something in the _flipbook_content_type_installed_instances arrays on the .install file.
Maybe you should check again the Field api to have a correct formated array.
http://api.drupal.org/api/drupal/modules!field!field.module/group/field/7

2- Even if it's not really a bug I would suggest to use explicit comments when committing changes. That can be very helpful to roll back if needed. Last 5 commits have all the same comment "fix code styles."

3- The only improvement I see from flippage module is the fact that your module shows the book inside a block instead of a page. Is that difference enough to create a new module ? It wouldn't be a better invested effort if you collaborate into the flippage project adding that feature ?

4- When setting the block on a template region I have the following errors:

    Warning: include(/var/www/d7/sites/all/modules/flipbook_content_type/flipbook.tpl.php): failed to open stream: No such file or directory in theme_render_template() (line 1495 of /var/www/d7/includes/theme.inc).
    Warning: include(): Failed opening '/var/www/d7/sites/all/modules/flipbook_content_type/flipbook.tpl.php' for inclusion (include_path='.:/usr/share/php:/usr/share/pear') in theme_render_template() (line 1495 of /var/www/d7/includes/theme.inc).

5- The module is calling _flipbook_installed_fields when the function name is defined as _flipbook_content_type_installed_fields. Check other name calls as I think you have mixed up flipbook_ with flipbook_content_type_ names.