WordPress.org

Make WordPress Themes

Opened 17 months ago

Closed 16 months ago

Last modified 16 months ago

#10283 closed theme (not-approved)

THEME: Sumobi Book - 1.0

Reported by: sumobi Owned by: chipbennett
Priority: Keywords: theme-sumobi-book
Cc: andrew@…

Attachments (2)

sumobi-book-lite.zip (781.8 KB) - added by sumobi 16 months ago.
passwordform.png (30.3 KB) - added by sumobi 16 months ago.

Download all attachments as: .zip

Change History (9)

comment:1 chipbennett16 months ago

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

comment:2 chipbennett16 months ago

  • Resolution set to not-approved
  • Status changed from assigned to closed
  • Theme URI is not appropriate (must be more than merely a demo)
  • Modify HTML document title content via the wp_title filter, rather than hard-coding content in the <title> tag, which must contain only a call to wp_title().
  • Modifying the WP login form is Plugin territory
  • All functional code in functions.php must be wrapped in a callback and hooked into an appropriate action hook

This is not a complete review.

comment:3 sumobi16 months ago

Thanks Chip,

I've attached a newer version which has been worked on since late nov/early december so there's a lot changed. I'm hoping this rectifies the issues mentioned. I couldn't see where I was modifying the login form (I always leave this to a plugin), and I didn't quite understand your comment about functional code, could you please elaborate in case I still haven't fixed this in the latest version? I've updated the theme URI and soft launched my website which now has a dedicated page for the theme, with links to demo and documentation. I've also fixed the wp_title() function, mimicking that of twenty twelve. http://sumobi.com/downloads/sumobi-book-lite/

comment:4 sumobi16 months ago

I'd also like to include a blank Child Theme but it pushed the file size over 1mb. Is this something I could add after it has been approved? Is it ok if I included the child theme within the main theme folder with instructions on how to install it? I think we should really be pushing making changes from a child theme, rather than the parent.

I've also changed the theme name to "sumobi-book-lite" as I have a commercial version available on my website which has slightly more features, such as added styles, priority support. Is it ok to include a link from within the theme options to the commercial version or is this too intruding to the user?

Last edited 16 months ago by sumobi (previous) (diff)

sumobi16 months ago

comment:5 chipbennett16 months ago

Note: you'll need to re-upload the revised Theme. Unfortunately, we can't work with ticket attachments. Our infrastructure isn't set up to work that way. Just upload the revised Theme, and the uploader will create a new ticket for it.

Ah... I should have said password form, in reference to this filter:

/**
 * Tweak password form
 */
function sumobi_modify_password_form() {

	global $post;

	$label = 'pwbox-'.( empty( $post->ID ) ? rand() : $post->ID );

	$o = '<div class="drop-shadow curved">';
	$o .= '<form class="protected-post-form" action="' . get_option('siteurl') . '/wp-login.php?action=postpass" method="post">';
	$o .= '<p>' . __( 'This post is password protected. To view it please enter your password below:', 'sumobi' ) .'</p>';
	$o .= '<p><label class="pass-label" for="' . $label . '">' . __( 'Password:', 'sumobi' ) . ' </label>';
	$o .= '<input class="text" name="post_password" id="' . $label . '" type="password" /></p>';
	$o .= '<button class="submit btn"><i class="icon icon-lock large"></i><span class="text">' . esc_attr__( 'Unlock' ) .'</span></button>';
	$o .= '</form></div>';

	return $o;
}
add_filter( 'the_password_form', 'sumobi_modify_password_form' );

Regarding functional code inside callbacks, for example:

/**
 * Set max width for media in content area. Used to assign a maximum width for images and embedded videos when inserting them into the content area
 */
 if ( !isset( $content_width ))
 	$content_width = 800;

Just move that inside your setup function, and globalize $content_width before you reference it:

global $ontent_width;
if ( !isset( $content_width ))
 	$content_width = 800;

Regarding the child Theme: I absolutely agree that it is beneficial to encourage end users to make modifications via Child Theme.

Out of curiosity: why do you include so many files (including image files) in your example Child Theme? I would suggest that you just include the requisite style.css, and maybe a blank functions.php.

Another alternative would be to host the Child Theme at your Theme URI site (and the updated URL looks just fine, FYI).

Another thing: since this is an upsell Theme, can you list somewhere the differences between the free and commercial versions?

comment:6 sumobi16 months ago

Right gotcha, thanks for the clarification. I think I'll do as you suggested and host the child theme on my site then. I'll be sure to list the changes between the two to make things a lot clearer. I modified the password form so I could wrap that extra <div> around it to match the styling of all the other forms. Is there another way I could do it perhaps?

I've included the images folder again as you've noticed because I'm referencing them using get_stylesheet_directory_uri(). I'd like the user to be able to modify the images through the use of the .PSD file (in the commercial version) or simply by opening up the 3 images in Photoshop and using a hue/saturation slider to adjust the colours. This means they don't have to touch the parent theme's images and can have their own custom modifications to the images, in the same way as they can modify files. I could probably make use of file_exists and check if there are images in the child theme first, if not use the parent's? this would allow me to remove these images from the child theme. What do you think?

I've also had to include the fonts folder which has the custom webfont for all the icons. This is because the fontface is being referenced from within the main stylesheet (style.css) and without it, the icons won't show. I've also included a "typography" folder which has the 2 typography presets. I don't believe in the "pick a font from the dropdown" route as each typeface needs special treatment with line-height, font-size, letter spacing etc. This way, a user can copy and paste one of the other typography files and make their own with minimal effort. Any .css files in this folder will show up in the theme options so they can select it from the dropdown. I plan on documenting this better.

comment:7 sumobi16 months ago

Not having much luck with the password form. I can't seem to justify making a plugin just to add a css class and an extra div to the form. It's also clearly filterable from post-template.php on line 1230 with the use of apply_filters. Should I just leave it with default styling then? I've attached a screenshot of it styled, and without.

sumobi16 months ago

Note: See TracTickets for help on using tickets.