WordPress.org

Make WordPress Themes

Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#23431 closed theme (live)

THEME: Wishbone - 1.04

Reported by: wishypw Owned by: jcastaneda
Priority: theme update Keywords: theme-wishbone
Cc: paul@…

Description

Wishbone - 1.04

Wishbone is a multi purpose WordPress theme containing everything needed to build a great WordPress website. Wishbone fully supports all WordPress theme features without being cluttered with gimmicky extras.

Theme URL - http://wishbone.wishynet.co.uk
Author URL - http://www.wishynet.co.uk

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

Diff with previous version: https://themes.trac.wordpress.org/changeset?old_path=/wishbone/1.03&new_path=/wishbone/1.04

History:

Ticket Summary Status Resolution Owner
#21649 THEME: Wishbone - 1.02 closed live jancbeck
#23189 THEME: Wishbone - 1.03 closed live emiluzelac
#23431 THEME: Wishbone - 1.04 closed live jcastaneda

(this ticket)

#24111 THEME: Wishbone – 1.06 closed live jcastaneda


https://themes.svn.wordpress.org/wishbone/1.04/screenshot.png

Change History (5)

#1 @jcastaneda
3 years ago

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

#2 @jcastaneda
3 years ago

  • Resolution set to live
  • Status changed from reviewing to closed

review based on diff view and changelog; all changes made accordingly, no issues to note.

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


3 years ago

#4 @jcastaneda
3 years ago

header.php

  • missing translation: L76, L106
  • missing theme_location in wp_nav_menu
  • duplicate <title> being created

index.php

  • no issues

footer.php

  • No need to check if dynamic_sidebar exists
  • can use the_widget

functions.php

  • missing translations for sidebar names
  • please use ID when registering sidebars
  • please register a menu ( wp_nav_menu being used in two locations )
  • favicon if used must be off by default and user configurable. ( L159 ) read more
  • L261 dash logo?
  • L287 please use require_once get_template_directory() . 'customizer.php';
  • L324 what is being changed?
  • unused custom_dash_logo hook ??

category.php

  • L24 no file exists loop-category.php
  • missing translation: L40, L44, L48,
  • the_time( 'F j, Y' ) please use the_time( get_option( 'time_format' ) )

comments.php

  • missing translation: L4
  • protected page/post password_required is missing.

content-*.php

  • missing wp_link_pages
  • missing translations: comments_number, just before the_category, the_tags
  • use the_time( get_option( 'time_format' ) )

page.php

  • missing wp_link_pages

One of the things I noticed was the use of a filter for the post_gallery. Great if you are creating your own version but the way you appear to be using it creates a PHP error. Not always but some of the time. The error is an undefined index columns error in particular. The reason I bring this up is because you can actually filter the shortcode attributes to suit your needs. For example:

<?php
add_filter( 'shortcode_atts_gallery', 'wishbone_gallery_sizes' );
function wishbone_gallery_sizes( $attr ){
        // Set initial image size
        $attr['size'] = 'wishbone-gallery';

        // set smaller than 3 columns
        if  ( $attr['columns'] == 1 ) {
                $attr['size'] = 'full';
        }
        
        if  ( $attr['columns'] == 2 ) {
                $attr['size'] = 'wishbone-gallery-wide';
        }
        return $attr;
}

Not only is this cleaner, you don't have to maintain as much code either. Another thing I noticed in the footer you are to an extent also recreating simple widgets. Why not do something like:

<?php
if ( !dynamic_sidebar( 'Footer 4' ) ) :
the_widget( 'WP_Widget_Text', array( 
    'title' => __( 'A text widget', 'text-domain' ), 
    'text' => __( 'This is some sample text that you can use within a text widget. It can use <strong>HTML markup</strong> <em>if</em> you want.', 'text-domain' ) 
    );
endif;

Another thing that raises a flag is your use of get_template_part( 'loop' ) or similar. It calls on some non-existent files like loop-category, loop-tag, nav-page, and section-slider.


There are also a lot of unused files along with a lot of commented code. Comments should really be used for documentation purposes rather than not letting code be executed.


In your JavaScript files you are doing:

var $ = jQuery;

It's a terrible practice, simply use jQuery or you can use something like:

jQuery(document).ready(function ($) {
        $('img.attachment-thumbnail').parent('a').addClass('thickbox');
        $('img.attachment-wishynet-gallery').parent('a').addClass('thickbox');
});

Be sure to update your translation files as well. :) Once you do, please link to the ticket here and I'll continue.

Any questions please be sure to ask.

#5 @wishypw
3 years ago

@jcastaneda hey, sorry for falling out of the loop, life got a little.. strange.

Anyway, I've made the updates you have suggested - thank you so much for the Gallery filtering advice, I'd been trying to figure out a very similar solution for ages but hadn't realised that I needed to filter the shortcode attributes rather than the gallery itself!

https://themes.trac.wordpress.org/ticket/24111

Last edited 3 years ago by wishypw (previous) (diff)
Note: See TracTickets for help on using tickets.