Future Look [D7] Responsive theme - 7.x

Git clone command for the sandbox is missing in the issue summary, please add it.

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.

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

Fixed the git clone URL in the issue summary for non-maintainer users.

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

/**
 * Future_look_textfield.
 */

Don't just repeat the function name in the docs -- that's pointless.

For theme overrides, there doesn't seem to be a clear standard, as Bartik and Garland differ in what they do, but Garland and Seven both do this:

/**
 * Override of theme_breadcrumb().
 */

For hook implementations such as future_look_js_alter(), see the documentation standards.

if (module_exists("webform")) {

You don't need to do this. If a module isn't enabled, then any theme overrides of its theme functions will just be ignored.

Hi Dev_irfan

Thanks for your contribution

There are indentation problem in page.tpl.php and use comment in info file for define style , script and feature.

Thanks

Hi Dev_irfan

Thanks for contributing a nice customizable theme from the UI which gives user a lot to customize.

The theme looks preety good when installed . But some part needs little more customization :

1. Some minor css change is needed for the responsiveness as the image which i added as an article for the content is not aligned properly when viewed in small screens.
2. Padding is needed for the for the article image and text.

Kindly check the attached screenshot image.
Thanks !

Overall, everything worked as I expected. The out-of-the-box slider widget is a nice feature.

Coding style & Drupal API usage

1. (*) theme_settings.php line 661:

   // Fron page slider Image upload and save permanetly . using submit function.

   It is unclear what this comment refers to or why it is here.

2. (*) theme_settings.php line 662:

   /**
    * Implements hook_form_submit().
    */
   

   There is no hook called "hook_form_submit()". You should actually document what this function does.

3. (*) theme_settings.php lines 668-715

   You repeat the same file saving actions 8 times here in nearly identical chunks:

     if (isset($form_state['values']['front_slide6'])&&$form_state['values']['front_slide6'] != 0) {
       $slide6_image = file_load($form_state['values']['front_slide6']);
       $slide6_image->status = FILE_STATUS_PERMANENT;
       file_save($slide6_image);
       file_usage_add($slide6_image, 'user', 'user', $user->uid);
     }
     if (isset($form_state['values']['bgimage'])&&$form_state['values']['bgimage'] != "") {
       $intro_region_image = file_load($form_state['values']['bgimage']);
       $intro_region_image->status = FILE_STATUS_PERMANENT;
       file_save($intro_region_image);
       file_usage_add($intro_region_image, 'user', 'user', $user->uid);
     }
   

   You could easily use a helper function that would reduce the complexity and length of the script.

4. (*) theme_settings.php lines 779-1035

   This huge chunk of inline CSS would be better in a template:

     $stylesheet .= '
     a{
       color:#' . $link_color . ';
     }
     body {
       background:#' . $body_bg_color . ' repeat url(' . $body_bg_image_url . ');
       color:#' . $body_txt_color . ';
       font-family:' . $font_family . ';
       font-size:' . $font_size . ';
     }';
     $stylesheet .= "
     .dropdown-menu{
       background-color:#" . $menu_text_color . ";
       font-size:" . $font_size . " ;
     }
     "
     ...
   

   If you used a template, you wouldn't need the huge chunk of variable-setting code starting on line 717. Instead you could pass a modified version of $form_state['values'] directly to the template.

5. (*) template.php

   You've got bootstrap file URLs hard-wired into functions:

   Line 83:
   $bootstrap_js_path = 'https://maxcdn.bootstrapcdn.com/bootstrap/3.3.5/js/bootstrap.min.js';
   Line 467
   $bootstrap_min = "https://maxcdn.bootstrapcdn.com/bootstrap/3.3.5/css/bootstrap.min.css";

   These should probably be constants declared at the top of the file.

6. (*) /admin/appearance/settings/future_look

   While some of these settings are obvious, some of them aren't and need descriptive text or instructions. We shouldn't have to watch a youtube video to understand what "Box 2 Settings" will do.

The starred items (*) are fairly big issues and warrant going back to Needs Work.

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.