The Panel Parallaxe module offers parallaxe display for your project.

The Parallaxe contains an fixed image in the background and some text that moves over the background.

The module contains:

  • Content Type Parallaxe: to specify background image and text
  • A custom display type: Parallaxe display, to be used together with the content of type Parallaxe
  • 8 different Panel Layouts, each containing three areas for Parallaxe display
    (Similiar projects: https://www.drupal.org/project/panels_extra_layouts)
  • 14 image styles to fit the backgound image best to screen-resolution

Project Page: https://www.drupal.org/sandbox/slowflyer/2426021

git clone --branch 7.x-1.x git@git.drupal.org:sandbox/slowflyer/2426021.git panel_parallaxe

When starting with the project I was looking for an all in one solution to get this kind of display, but could'nt find it. There are themes out in the universe where you can add manually a parallaxe block to your site. Which makes it hard to change and maintain.
With the use of the module you can either add your parallaxe content direct to panel page, or you can use views, nodequeues, ... to select dynamically one parallaxe content to display.

Compared to:

  • Parallax Toolkit: Mobile devices and smaller screensizes are supported with 14 defined image styles for perfect fit
  • Parallax Background: Mobile devices and smaller screensizes are supported with 14 defined image styles for perfect fit, no extra external js needed

Manual reviews of other projects

[D7] Responsive Blocks
[D7] Table API
[D7] Super Monitoring

Files: 
CommentFileSizeAuthor
#16 parallex-warning.png35.6 KBnaveenvalecha

Comments

PA robot’s picture

Status: Needs review » Needs work

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

We are currently quite busy with all the project applications and we prefer projects with a review bonus. Please help reviewing and put yourself on the high priority list, then we will take a look at your project right away :-)

Also, you should get your friends, colleagues or other community members involved to review this application. Let them go through the review checklist and post a comment that sets this issue to "needs work" (they found some problems with the project) or "reviewed & tested by the community" (they found no major flaws).

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

slowflyer’s picture

Status: Needs work » Needs review

Done.
clean.

vbouchet’s picture

Hi,

Please find my findings. All may not be applicable but ...

You miss the git command to clone the repo.

  • panel_parallaxe_init()
    • if (strpos(current_path(), '/admin/') === FALSE) {...} can probably be replace by if (!path_is_admin(current_path())) {...}
    • drupal_add_css(), CSS_DEFAULT is the default group value so it's not required to add it.
  • panel_parallaxe_menu()
    • I don't get how it works as the page_argument is "para_admin_form" but the function name in admin.inc is "panel_parallaxe_admin_form".
  • panel_parallaxe_admin.inc should probably be panel_parallaxe.admin.inc
  • panel_parallaxe_uninstall()
  • js and css files should be named panel_parallaxe.js, panel_parallaxe.panel.css, panel_parallaxe.para.css (or -panel.css & -para.css)
  • JS is not following Drupal rules (should use behaviors)

Thanks,

vbouchet’s picture

Status: Needs review » Needs work
slowflyer’s picture

Issue summary: View changes
slowflyer’s picture

Issue summary: View changes
slowflyer’s picture

Status: Needs work » Needs review

@vbouchet: Thanks for your valuable feddback!

All findings should be solved.

slowflyer’s picture

Issue summary: View changes
slowflyer’s picture

Issue summary: View changes
slowflyer’s picture

Issue summary: View changes
D2ev’s picture

I would suggest to use system_settings_form in admin settings beside that the code looks good and I haven't test the module functionality yet.

slowflyer’s picture

@D2ev: Thanks a lot, I followed your suggestion.

slowflyer’s picture

Issue summary: View changes
Issue tags: +PAReview: review bonus
cfischer83’s picture

Status: Needs review » Reviewed & tested by the community

Automated Review

[Best practice issues identified by pareview.sh: None found: http://pareview.sh/pareview/httpgitdrupalorgsandboxslowflyer2426021git

Manual Review

Individual user account
Yes Does not follow the guidelines for individual user accounts.
No duplication
Yes: Does not cause module duplication and/or fragmentation.
Master Branch
Yes: Follows the guidelines for master branch.
Licensing
Yes: Follows the licensing requirements.
3rd party assets/code
Yes: Follows the guidelines for 3rd party assets/code.
README.txt/README.md
Yes: Follows the guidelines for in-project documentation and/or the README Template.
Code long/complex enough for review
Yes: Follows the guidelines for project length and complexity.
Secure code
Yes: Meets the security requirements. / No: List of security issues identified.]
Coding style & Drupal API usage
[List of identified issues in no particular order. Use (*) and (+) to indicate an issue importance. Replace the text below by the issues themselves:
  • One suggestion that’s more of a feature request than a deal breaker (although as a Drupal developer, personally it would be a deal breaker for me). Consider using a new entity type rather than a “Parallaxe” Node. For example, you could use a fieldable panel pane: https://www.drupal.org/project/fieldable_panels_panes
    There are certain implications that go along with using a node that you won’t get with a custom entity or FPP.
  • If you use a FPP, you can default things so you won’t have to remember the “build mode” like I kept forgetting.
  • I’ve spent a good amount of time in here and see no major issues!
  1. (*) Major finding, needs work
  2. (+) Release blocker
  3. Just a recommendation
  4. ...]

The starred items (*) are fairly big issues and warrant going back to Needs Work. Items marked with a plus sign (+) are important and should be addressed before a stable project release. The rest of the comments in the code walkthrough are recommendations.

If added, please don't remove the security tag, we keep that for statistics and to show examples of security problems.

This review uses the Project Application Review Template.

klausi’s picture

Assigned: Unassigned » naveenvalecha
Status: Reviewed & tested by the community » Needs work
Issue tags: -PAReview: review bonus +PAReview: security

Review of the 7.x-1.x branch (commit d707e00):

  • Coder Sniffer has found some issues with your code (please check the Drupal coding standards).
    FILE: /home/klausi/pareview_temp/js/panel_parallaxe.js
    --------------------------------------------------------------------------
    FOUND 21 ERRORS AFFECTING 21 LINES
    --------------------------------------------------------------------------
      2 | ERROR | [ ] The second line in the file doc comment must be "@file"
     10 | ERROR | [x] Expected 1 newline after opening brace; 0 found
     11 | ERROR | [x] Expected 1 newline after opening brace; 0 found
     12 | ERROR | [x] Expected 1 newline after opening brace; 0 found
     13 | ERROR | [x] Expected 1 newline after opening brace; 0 found
     14 | ERROR | [x] Expected 1 newline after opening brace; 0 found
     15 | ERROR | [x] Expected 1 newline after opening brace; 0 found
     16 | ERROR | [x] Expected 1 newline after opening brace; 0 found
     19 | ERROR | [x] Expected 1 newline after opening brace; 0 found
     20 | ERROR | [x] Expected 1 newline after opening brace; 0 found
     21 | ERROR | [x] Expected 1 newline after opening brace; 0 found
     22 | ERROR | [x] Expected 1 newline after opening brace; 0 found
     23 | ERROR | [x] Expected 1 newline after opening brace; 0 found
     24 | ERROR | [x] Expected 1 newline after opening brace; 0 found
     25 | ERROR | [x] Expected 1 newline after opening brace; 0 found
     28 | ERROR | [x] Expected 1 newline after opening brace; 0 found
     29 | ERROR | [x] Expected 1 newline after opening brace; 0 found
     30 | ERROR | [x] Expected 1 newline after opening brace; 0 found
     32 | ERROR | [x] Expected 1 newline after opening brace; 0 found
     33 | ERROR | [x] Expected 1 newline after opening brace; 0 found
     34 | ERROR | [x] Expected 1 newline after opening brace; 0 found
    --------------------------------------------------------------------------
    PHPCBF CAN FIX THE 20 MARKED SNIFF VIOLATIONS AUTOMATICALLY
    --------------------------------------------------------------------------
    
  • No automated test cases were found, did you consider writing Simpletests or PHPUnit tests? This is not a requirement but encouraged for professional software development.

This automated report was generated with PAReview.sh, your friendly project application review script. You can also use the online version to check your project. You have to get a review bonus to get a review from me.

manual review:

  1. what are the differences to existing project such as https://www.drupal.org/project/parallax and https://www.drupal.org/project/parallax_bg ? Please add that to your project page.
  2. panel_parallaxe.info: why is there no dependency to panels if this is in the Panels package? Please add a comment.
  3. panel_parallaxe_preprocess_html(): why do you need the CSS and JS globally on every page? You should only add it if your content is displayed, for example in panel_parallaxe_node_view()?
  4. README.txt: why do people have to enable the insecure image_allow_insecure_derivatives setting, why can't you calculate the token?
  5. Notice: Undefined index: und in theme_parallaxe_view_theme() (line 86 of panel_parallaxe.module) if there is not image set.
  6. This module has a security vulnerability. As part of our git administrator training program i'm assigning this to Naveen so he can take look. If he does not find anything I'm going to post the details about this vulnerability in one week.

Removing review bonus tag, you can add it again if you have done another 3 reviews of other projects.

naveenvalecha’s picture

Assigned: naveenvalecha » Unassigned
FileSize
35.6 KB

@slowflyer,
Thanks for your contribution.
Manual Review:
+1 as Klausi specified.

  1. (+)theme_parallaxe_view_theme : You are passing the data without sanitization.Use the sanitization functions to sanitize the data before passing.Please see
  2. (*)I am getting the warnings as well.Please see the attached screenshot.
  3. panel_parallaxe.install : panel_parallaxe_install the variablename $node_example should be more specific like $parallaxe
  4. panel_parallaxe_preprocess_html : +@klausi and Also You are adding js/css files using drupal_add_js/drupal_add_css rather use #attached to add the JS/CSS file
    with vars rendered array and where these actually required.
  5. panel_parallaxe_menu : For long term I thing you should define the extra permission by your module and use that permission for this page.
  6. theme_parallaxe_view_theme : $parallaxe = image_style_url('parallaxe_l_1920_1080', $node->parallaxe[LANGUAGE_NONE][0]['uri']); It will work fine for non-multilingual websites,Use global language object and use the language in place of languge_none.
  7. use single quotes where possible for Drupal code standards and a very slight performance benefit.It is used at various places in module like panel_parallaxe_views_api.
  8. hook_help() is missing in this module.It would be nice to add it.

I would recommend you, please help to review other project applications to get a review bonus back. This will put you on the high priority list again, then git administrators will take a look at your project right away :)

slowflyer’s picture

Issue summary: View changes
slowflyer’s picture

Issue summary: View changes
slowflyer’s picture

Status: Needs work » Needs review

@klausi & @naveenvalecha: Thanks a lot for your feedback. Hope I got everything fixed mentioned from you. Only hook_help is not implemented so far.

naveenvalecha’s picture

Status: Needs review » Reviewed & tested by the community

Manual Review : Read 0789bd5...

  1. panel_parallaxe_install :
      foreach (_panel_parallaxe_installed_fields() as $field) {
        field_create_field($field);
      }

    Please add a check before that the field already exists using field_info_field so that it will not throw warnings.

  2. Please specify on your project page/Readme.txt on the uninstallation of the module it will remove the parallaxe bundle and its nodes.
  3. theme_parallaxe_view_theme : You are adding js/css files using drupal_add_js rather use #attached to add the JS/CSS file with $variables
  4. theme_parallaxe_view_theme : Please specify on the project page that the first image will be set as parallex image if use will change the configuration later of the image field and make it to upload multiple files.
  5. Adding the hook_help is nice

The above are not blockers.So RTBC :)
As all reviewers are currently quite busy with all the project applications and we prefer projects with a review bonus. Please help reviewing and put yourself on the high priority list, then we will take a look at your project right away :-)

naveenvalecha’s picture

Assigned: Unassigned » kscheirer

Assigning to kscheirer for final review if he has time.

slowflyer’s picture

No progress over 4 month?

naveenvalecha’s picture

Review of the 7.x-1.x branch (commit 0789bd5):

  • Coder Sniffer has found some issues with your code (please check the Drupal coding standards).
    
    FILE: ...www/d7/sites/all/modules/contrib/pareview_temp/panel_parallaxe.module
    ---------------------------------------------------------------------------
    FOUND 2 ERRORS AFFECTING 2 LINES
    ---------------------------------------------------------------------------
     92 | ERROR | [x] Closing parenthesis of array declaration must be on a
        |       |     new line
     99 | ERROR | [x] Closing parenthesis of array declaration must be on a
        |       |     new line
    ---------------------------------------------------------------------------
    PHPCBF CAN FIX THE 2 MARKED SNIFF VIOLATIONS AUTOMATICALLY
    ---------------------------------------------------------------------------
    
    Time: 1.17 secs; Memory: 11Mb
    
  • No automated test cases were found, did you consider writing Simpletests or PHPUnit tests? This is not a requirement but encouraged for professional software development.

This automated report was generated with PAReview.sh, your friendly project application review script. You can also use the online version to check your project. You have to get a review bonus to get a review from me.

No more objections from last 4 months and no commit after 0789bd5...
None address from #20
No more blocker.

naveenvalecha’s picture

Assigned: kscheirer » Unassigned
Status: Reviewed & tested by the community » Fixed

Thanks for your contribution, slowflyer!

I updated your account so you can 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 stay 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.

slowflyer’s picture

@naveenvalecha: Thanks! I fixed the 2 items from #23
But it seems I still can't promote it to full project.

naveenvalecha’s picture

oops my bad I forget one step.Please check again it will work for you.

slowflyer’s picture

Thanks a lot!
You made my day!

Status: Fixed » Closed (fixed)

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