WordPress.org

Make WordPress Themes

Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#24147 closed theme (not-approved)

THEME: King – 1.1.1

Reported by: brainstormforce Owned by: thelukemcdonald
Priority: previously reviewed Keywords: theme-king
Cc: mail.sujaypawar@…

Description

King - 1.1.1

The King theme is a simple & responsive theme, which is very flexible, lightweight & specifically for demo site of our “Ultimate Addons for Visual Composer” plugin.

Theme URL - http://brainstormforce.com/demos/king/
Author URL - https://www.brainstormforce.com/

SVN - https://themes.svn.wordpress.org/king/1.1.1
ZIP - https://wordpress.org/themes/download/king.1.1.1.zip?nostats=1

Diff with previous version: https://themes.trac.wordpress.org/changeset?old_path=king/1.1.0&new_path=king/1.1.1

History:

Ticket Summary Status Resolution Owner
#16852 THEME: Kingdom - 1.3 closed live salsaturation
#18285 THEME: King Church Theme - 1.1 closed not-approved emiluzelac
#18528 THEME: Kingdom - 1.4 closed live robin90
#19992 THEME: Kingdom - 1.5 closed live bijayyadav
#23482 THEME: King – 1.1.0 closed closed-newer-version-uploaded pratikchaskar
#24147 THEME: King – 1.1.1 closed not-approved thelukemcdonald

(this ticket)

#25248 THEME: kingbird – 2.5 closed not-approved laurelfulford
#34273 THEME: Kingdom – 1.6 closed live themetracbot
#35786 THEME: Kingdom – 1.7 closed live themetracbot
#48622 THEME: kingcabs – 1.0.1 new


https://themes.svn.wordpress.org/king/1.1.1/screenshot.png

Change History (5)

This ticket was mentioned in Slack in #themereview by nik. View the logs.


3 years ago

#2 @themetracbot
3 years ago

  • Owner set to thelukemcdonald
  • Status changed from new to reviewing

#3 @thelukemcdonald
3 years ago

Hi @brainstormforce,

I'll be reviewing the King theme v1.1.1. When reviewing, I'll add a list of required changes, if any, that will need to be made before the next theme review. I'll also add any recommendations and/or notes that that aren't required, but may be good practice or worth mentioning.

#4 @thelukemcdonald
3 years ago

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

Required

Required items that must be met before theme can be approved.

Code

  • Remove title tag in header.php; theme has already added support for the title tag in functions.php.
  • Update $site_width to $content_width in functions.php (assuming that is what you meant to do). Otherwise, $site_width does not appear to be used, where as [$content_width](https://codex.wordpress.org/Content_Width) is a core WP global.
  • Remove king_custom_css(). This is more along the lines of plugin terriotry, in which there are a few that do a really good job of it, and do it securely.
  • Remove king_masonry_blog() and enqueue scripts rather than printing to footer. Maybe move to functions.js?
  • Remove king_scroll_to_top() and enqueue scripts rather than printing to footer. Maybe move to functions.js?
  • Remove king_custom_script(). This is more along the lines of plugin territory. It's also required that no scripts be hardcoded, and I would think this would qualify.
  • Remove scripts from king_gallery_shortcode() and enqueue them rather than printing to footer.
  • Remove scripts in king_justified_grid_gallery() and enqueu them rather than printing to footer.
  • Update @package to use theme name (CamelCase), not WordPress.
  • Remove @subpackage with theme name from docblocks.
  • Add @package to the docblock at the top of each template file.

Documentation

  • There is a number of custom setting and featurs included in the theme. Some documentation is required.

Language

  • Localize $title strings in king_title_breadcrumb_bar().
  • Localize text and attribute strings in king-widget.php.
  • Localize all default text, labels, and descriptions in customizer.php.

Licensing

Licensing can generally be added to the readme.txt file.

  • Add licensing for Open Sans font.
  • Add licensing for Font Awesome font icons.

Options and Settings

  • Remove king_add_toolbar_items() as a link to the Customizer is already provided under the Appearance menu. This helps keep the admin menu clutter free.

Plugins

  • Remove typography settings (class-King_Typography_Controls.php). This would be considered plugin territory as there are a number of plugins out there to handle fonts for Google, TypeKit, and more.
  • Remove favicon functionality and settings. This is better added via a plugin so the favicon can be used irregardless of the theme being used. See king_favicon().

Security and Privacy

  • Escape urls and attributes in king_search_form()
  • Escape urls and text in king_number_pagination()
  • Escape ID attribute in king_content_nav().
  • Escape url, id, class, and title attributes in king_gallery_shortcode().
  • Escape class attribues in content-blog-medium.php.
  • Escape url, title, and class attribues in footer.php.
  • Escape all attributes in king-widget.php (the majority of attibutes need escaped in this file).
  • Escape all attributes and text in class-King_Typography_Control.php.
  • Escape attributes and text in class-King_Textarea_Control.php.
  • Escape attributes and text in vc_row.php.
  • Remove empty localization strings king-widget.php.
  • Properlay sanitize all Customizer settings. The king_sanitize_callback() just returns the submitted value and doesn't sanitize it at all. Sanitizing the value makes sure that the value is of an expected result, so each one may need a specific sanitization function applied. See the codex information on Data Sanitization and this Theme Shaper article for me info.

Stylesheets and Scripts

  • Remove minified scripts and styles.

Recommended

Not required but good practice.

Core Functionality and Features

  • Consider adding more stylesheet tags to the theme stylesheet to better define its features and attributes.

Code

  • Consider registering menus in king_navigation_menus() using the after_theme_setup hook, or, just add the menu regisgration inside king_setup().
  • Consider adding additional image sizes via the king_setup() function.
  • Consider alternative ways to get audo/video from content, or, atleast cache the results.
  • Consider using the_post_navigation() function to output post navigation. See king_single_post_navigation()
  • Consider using 'the_posts_navigaiton()' to output archive navigations. See king_content_nav()
  • Consider using 'the_posts_pagination()' to output archive navigations. See king_number_pagination()
  • Consider using the_archive_title() function to output archive titles. See king_archive_header_text()
  • Consider updating _e() and _ex() to use esc_html_e() and esc_html_ex(). These will be part of the updated WordPress Coding Standards.
  • Consider adding support for a breadcrumbs plugin (e.g. WordPress SEO by Yoast breadcrubms) rather than implmenting a custom solution. See king_breadcrumb()
  • Consider removing function_exists() wrappers around functions that invokes via an action hook. Child themes can remove an action and add their own solution if needed.
  • Consider ommitting closing PHP tag at the end of a file per WordPress Coding Standards.
  • Consider removing custom design integration. This may be required in a future review. There is a good amount of code here that could be simplified or removed if support was added for a plugin, in which this is plugin territory to a certain degree. You might also look into how Twenty Fifteen goes about adding custom styles. It can be tough when trying to properly escape CSS.

Stylesheets and Scripts

  • Scripts and styles are automatically registered when using wp_enqueue_style(), so there isn't a need to define a separate wp_register_style() followed by an wp_enqueue_style().
  • Consider reducing the number of enqueued scripts. Maybe combine scripts into one file?
  • Consider enqueueing scripts as they are needed. For example, not all pages may need to have the slider script(s) loaded.

Notes

Design notes and any additional information.

  • It appears the theme adds support for WooCommerce, however, it doesn't appear to customize the plugin or styles in any way.
  • It's best to escape attributes and text as late as possible. In the Required notes above, I've mentioned escaping attributes. In some cases, not all, the attributes were escaped early on, but should be escaped when printed to the page when possible.
  • The theme version in the stylesheet should match the uploaded zip version.

#5 @thelukemcdonald
3 years ago

Hi @brainstormforce,

Above are required, recommended, and notes for the King theme. The required notes will need to be addressed before uploading a new version. At that time, the theme will be reviewed again and any additional notes will be made. Let me know if you have any questions.

Note: See TracTickets for help on using tickets.