Opened 8 months ago

Closed 8 months ago

Last modified 8 months ago

#9433 closed theme (not-approved)

THEME: WP Strap - 0.4.1

Reported by: gbuckingham89 Owned by: garinungkadol
Priority: major Keywords: theme-wp-strap
Cc: george@…

Description

WP Strap - 0.4.1

WP Strap is an open-source WordPress? theme using the Bootstrap framework from Twitter. The aim of this theme is to use as much of the default Bootstrap styling as possible, with only minor CSS additions to provide styling for WordPress? features. It is an on-going project and will continue to be expanded as well as updated to support the latest versions of Bootstrap and WordPress?.

Theme URL - http://wpstrap.com/
Author URL - http://georgebuckingham.com/

SVN - http://themes.svn.wordpress.org/wp-strap/0.4.1
ZIP - http://wordpress.org/extend/themes/download/wp-strap.0.4.1.zip?nostats=1

All previous tickets for this theme: http://themes.trac.wordpress.org/query?keywords=~theme-wp-strap

http://themes.svn.wordpress.org/wp-strap/0.4.1/screenshot.png

Attachments (7)

content-table.jpg (13.7 KB) - added by garinungkadol 8 months ago.
lists-in-comments.jpg (23.6 KB) - added by garinungkadol 8 months ago.
navigation-menu.jpg (12.2 KB) - added by garinungkadol 8 months ago.
menu-multi-line.jpg (56.9 KB) - added by garinungkadol 8 months ago.
menu-one-line.jpg (53.4 KB) - added by garinungkadol 8 months ago.
header-search-form.jpg (10.8 KB) - added by garinungkadol 8 months ago.
post-meta.jpg (1.6 KB) - added by garinungkadol 8 months ago.

Download all attachments as: .zip

Change History (14)

comment:1 Changed 8 months ago by kobenland

Hi George,

just FYI, the Apache 2.0 license is not compatible with GPL v2.

Most of what you do in inc/options.php is covered by the Settings API. You might want to look into functions like add_settings_section() and add_settings_field()

Themes are required to use core-bundled scripts, if using such scripts (like jQuery)

This was not a review.

Konstantin

comment:2 follow-up: Changed 8 months ago by gbuckingham89

There are other themes that use Bootstrap (which has the Apache license) such as: http://wordpress.org/extend/themes/the-bootstrap.

Thanks for the heads up about add_settings_section() and add_settings_field() - they aren't somethingI've come across before, so will look into them in the future.

comment:3 in reply to: ↑ 2 Changed 8 months ago by kobenland

Replying to gbuckingham89:

There are other themes that use Bootstrap (which has the Apache license) such as: http://wordpress.org/extend/themes/the-bootstrap.

That is correct. It is released under GPL v3, though.

comment:4 Changed 8 months ago by garinungkadol

  • Owner set to garinungkadol
  • Status changed from new to assigned

Changed 8 months ago by garinungkadol

Changed 8 months ago by garinungkadol

Changed 8 months ago by garinungkadol

Changed 8 months ago by garinungkadol

Changed 8 months ago by garinungkadol

Changed 8 months ago by garinungkadol

Changed 8 months ago by garinungkadol

comment:5 Changed 8 months ago by garinungkadol

  • Resolution set to not-approved
  • Status changed from assigned to closed

General Info

  • REQUIRED: GPL version 2 is not compatible with the Apache license. You must license your theme under GPL 3 license.

* REQUIRED: You cannot include the glyphicons because they are licensed under CC BY 3.0 which is incompatible with GPL.

Debug Errors

On Site after theme is first activated

  • Errors disappear when a Navigation Menu is assigned.

-- Debug: Undefined property: stdClass::$db_id on line 363 of C:\xampp\htdocs\wordpress\themereview\wp-content\themes\wp-strap\inc\theme.php
-- Debug: Trying to get property of non-object on line 300 of C:\xampp\htdocs\wordpress\themereview\wp-content\themes\wp-strap\inc\theme.php
-- Debug: Undefined property: stdClass::$current on line 301 of C:\xampp\htdocs\wordpress\themereview\wp-content\themes\wp-strap\inc\theme.php
-- Debug: Undefined property: stdClass::$current_item_ancestor on line 301 of C:\xampp\htdocs\wordpress\themereview\wp-content\themes\wp-strap\inc\theme.php
-- Debug: Trying to get property of non-object on line 329 of C:\xampp\htdocs\wordpress\themereview\wp-content\themes\wp-strap\inc\theme.php
-- Debug: Trying to get property of non-object on line 335 of C:\xampp\htdocs\wordpress\themereview\wp-content\themes\wp-strap\inc\theme.php
-- Debug: Trying to get property of non-object on line 339 of C:\xampp\htdocs\wordpress\themereview\wp-content\themes\wp-strap\inc\theme.php
-- Debug: Trying to get property of non-object on line 341 of C:\xampp\htdocs\wordpress\themereview\wp-content\themes\wp-strap\inc\theme.php
-- Debug: Trying to get property of non-object on line 342 of C:\xampp\htdocs\wordpress\themereview\wp-content\themes\wp-strap\inc\theme.php
-- Debug: Trying to get property of non-object on line 343 of C:\xampp\htdocs\wordpress\themereview\wp-content\themes\wp-strap\inc\theme.php
-- Debug: Undefined property: stdClass::$db_id on line 374 of C:\xampp\htdocs\wordpress\themereview\wp-content\themes\wp-strap\inc\theme.php
-- Debug: Undefined property: stdClass::$db_id on line 363 of C:\xampp\htdocs\wordpress\themereview\wp-content\themes\wp-strap\inc\theme.php

On Options Page
-- Debug: Undefined index: label on line 427 of C:\xampp\htdocs\wordpress\themereview\wp-content\themes\wp-strap\inc\options.php

Theme Unit Test

Blog Index View

  • REQUIRED: Posts with no title must still include the permalink to the single post view.

Single Post View

  • Is this the intended styling for table tags when used in posts? See content-table.jpg

Navigation Menus

  • REQUIRED: Check how you have implemented the navigation menu links. I am unable to click on the top most links if it has a child link below it. See navigation-menu.jpg
  • REQUIRED: If a navigation menu consists of multiple lines, part of the content is hidden. See menu-multi-line.jpg versus menu-one-line.jpg. If this cannot be fixed, place a note in the readme.txt encouraging users to have only one line of links.

Others

  • RECOMMENDED: A bit more space between the header search form and the edge of the header box. See header-search-form.jpg.
  • RECOMMENDED: When the "Small" sidebar option is selected, in 1024x768 resolution, a horizontal scrollbar appears.
  • RECOMMENDED: In the Post Options, if "Show post date / time" is set to "No", the vertical bar separating date/time display and author still shows. See post-meta.jpg

Functionality

  • REQUIRED: The "Show comments list & form" option for Pages needs to be re-worked. The general requirements are:
    • If a page has not enabled comments, the page must not show any kind of "Comments are closed" message
    • If a page has comments are enabled, the comments list and form must be displayed.


  • REQUIRED: What is the purpose of the "Show comments list & form" option for Posts? At present it is a catch-all option to disable commenting and the display of comments? Is this the intention?
  • REQUIRED: The 'comment-reply' script is not working properly. You should enqueue the script inside a function hooked with the appropriate hook e.g. wp_enqueue_scripts or comment_form_before.
  • RECOMMENDED: "Show old post" should be disabled by default

Code Quality

Theme Settings

  • REQUIRED: If you don't intend to use the Settings API you must include:
    • Nonces in your options form. See WordPress Nonces
    • Validate the nonce before proceeding to your save routine.
    • You must validate/sanitize $_POST data before updating options.
  • REQUIRED: Themes are required to use esc_attr() for text inputs and esc_html()/esc_textarea() for textareas.


  • REQUIRED: Themes are required to save options in a single array, rather than create multiple options for its settings page.
  • REQUIRED: Themes are required to include within the Theme all images, scripts, and other bundled resources. Such resources must not be "hotlinked" from a third-party site
    • HTML5 shiv should be included in the theme's package
    • Place the code in a function that hooks in to wp_head.
  • REQUIRED: Themes are required to use core-bundled scripts, if using such scripts
    • Don't include jQuery with your theme package. Instead make use of jQuery that WordPress? uses.
  • REQUIRED: You must use get_search_form() to call the search form. To control the what type of search form to display, either use the searchform.php template or the action hook 'get_search_form'.
  • REQUIRED: Include the development version for all minified scripts for reference purposes.
  • RECOMMENDED: The options page uses the same functionality as Settings API. You may want to consider using that instead.

REVIEW SUMMARY

  • Performed a complete review
  • Resolving ticket as not-approved due to above issues.
  • All issues tagged as required and debug errors must be fixed in the next release.

Before re-submitting your theme, check it against the Theme Review guidelines and Theme Unit Test. The following tools will be of use during this process:

If you have any questions about this review, feel free to ask here.

~ Vicky

Last edited 8 months ago by garinungkadol (previous) (diff)

comment:6 follow-up: Changed 8 months ago by kobenland

Hi Vicky,

just a heads-up:
As of recently, as part of Bootstrap, Glyphicons are also Appache 2.0 licensed. See this thread for reference.

Thanks,
Konstantin

comment:7 in reply to: ↑ 6 Changed 8 months ago by garinungkadol

Replying to kobenland:

just a heads-up:
As of recently, as part of Bootstrap, Glyphicons are also Appache 2.0 licensed. See this thread for reference.

Thanks Konstantin for the FYI.
@gbuckingham89, I've revised the review to reflect this.

Note: See TracTickets for help on using tickets.