WordPress.org

Make WordPress Themes

Opened 7 months ago

Closed 5 months ago

#41409 closed theme (not-approved)

THEME: Shadower – 2.5

Reported by: uiuxlab Owned by: timph
Priority: previously reviewed Keywords: theme-shadower
Cc: uiuxlab@…

Description

Shadower - 2.5

Shadower is a beautiful and full responsive blog WordPress theme for displaying topics and artworks. It is easy to use and customize, with a clean, simple, and modern design, perfect for bloggers. Theme has many other customisation options too, including changing the typography, layout, widgets, advertising, slides and so on. To suit you and your audience.

Theme URL - https://uiux.cc/wp-themes/shadower/
Author URL - https://uiux.cc

Trac Browser - https://themes.trac.wordpress.org/browser/shadower/2.5

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

Diff with previous version: https://themes.trac.wordpress.org/changeset?old_path=shadower/2.0&new_path=shadower/2.5

History:

Ticket Summary Status Resolution Owner
#36289 THEME: Shadower – 1.0.0 closed not-approved poena
#39105 THEME: Shadower – 2.0 closed not-approved acosmin
#41409 THEME: Shadower – 2.5 closed not-approved timph

(this ticket)

#43053 THEME: Shadower – 2.7.6 closed live sushil adhikari
#46734 THEME: Shadower – 2.7.7 closed live themetracbot


https://themes.svn.wordpress.org/shadower/2.5/screenshot.png

Change History (7)

#1 @joyously
6 months ago

Here is some user feedback to consider before your review.

  • The menu needs to have a fallback for when no menu is selected. You have an empty fallback, and it is never called because you check for a menu first. Remove the if statement, remove all the empty parameters to wp_nav_menu(), and remove the fallback_cb parameter to use the core fallback page menu.
  • Comments do not have a link to the comment anchor (for ease of copy/paste). This is typically put on the date.
  • The post title and post meta on the single post page take up so much room that I have to scroll to see any content. This is very annoying.
  • The author bio box has a link to the author's posts, but it is not intuitive to find it. There is a barely visible arrow at the bottom and that container is extended all the way across, but it's just not logical compared to linking the name or the avatar (or both) or text that says "View all posts by xxx".
  • Formatting:
    • links in comments are barely different from regular text
    • <h6> is quite small
    • blockquote uses content:open-quote, but not close-quote, so the first one encountered has a double quote mark and the subsequent ones have a single quote mark, and it affects <q> also
    • blockquote is styled as italic, but can contain <em> and <cite> which are also italic, but you wouldn't be able to tell
    • tables in user content are styled, but the calendar widget is not
    • the bottom margin on <li> makes the list uncomfortably tall (can't make a short list!)
    • <hr> is barely visible
    • large images are distorted in narrow containers (use img {max-width:100%; height:auto} to fix)
    • Floats are not being cleared properly after the content area.
    • I really do not like the blur applied to the page when the menu is opened on a small screen.
    • The animation on the 404 page is obnoxious, and just might trigger seizures in those susceptible. The page is not helpful at all.
    • some of the footer widgets are centered, but not all. The ones I chose that are (menu, feed) should not be.
    • you might want to hide the menu and widget areas for the print styles
  • The category page puts a list of all the categories at the top, making those other categories more prominent than the content that is actually in the category being displayed.
  • The admin page to install recommended plugins still says "Required Plugins".
  • Theme is tagged theme-options, but does not supply any options.
  • Need to output pingback <link> in head section (conditional on singular and pings_open)
  • Remove the favicon folder and your logo used as default (no one wants your logo) and duplicates (look at assets vs. includes/admin/assets).
  • Remove define('SHADOWER_THEME_URL', get_template_directory_uri() ). This needs to be dynamic every time it is used so child themes and plugins can filter it. The define happens before all filters are set up, and it's constant.
  • Page templates should not start with 'page-' because of the template hierarchy. The file page-contact.php would be the template file used for a page with a slug of 'contact'.

#2 @kevinhaig
6 months ago

Note to Author

  1. The above preview by @joyously is not an official review. You must still wait to get to the top of the review queue before an official review will begin.
  1. Many of the concerns above relate to results from Theme Unit Test and you should always test your theme with that package. However please note that many of the comments are considered recommended. Recommended comments DO NOT have to be addressed for theme approval. In many cases it is good advice, but the decision to address those issues is yours.
  1. There may be some comment items that are required. Required items must be addressed or the theme will not be approved. Please visit https://make.wordpress.org/themes/handbook/review/required/ to double check required items. Note that when an official review starts your theme may be closed if 5 or more distinct requirements are not met.
  1. If you are in doubt of a comment, or do not want to address something please ask for a confirmation in the ticket, or on the open Slack channel.
  1. If I get a chance I will take a look later and split out the above comments between Requirements and Recommended as @joyously should be doing :(

#3 @uiuxlab
6 months ago

Thank you very much @kevinhaig @joyously , I will refer to the improvements and publish a new version that takes some time. I will make some improvement based on the official and your feedback. :)

If I could not complete it before the official review , I will post a new version based on your feedback. I love WordPress and hope to have your help can do it better.

Best Regards.

Last edited 6 months ago by uiuxlab (previous) (diff)

#4 @themetracbot
5 months ago

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

#5 @djrmom
5 months ago

  • Owner buffalokumc deleted

I am going to add this to the new queue again, as there has been no response yet from the reviewer. A reviewer should make an initial contact in ticket within 24 hours of being assigned, after which both reviewer and author should always communicate within 7 days.

If you are the reviewer and able to do this review, please carry on and request you get added back in Slack #themereview or you can take on another review when you have time again.

#6 @themetracbot
5 months ago

  • Owner set to timph

#7 @timph
5 months ago

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

Hey @uiuxlab,

Thanks so much for your submission! I have partially reviewed the theme, and see a few areas that should be addressed, and some others that I think would be helpful to fix. With five or more issues, the theme review will be closed, and once you've had an opportunity to address these, then please submit it back for another review!

Some Required Things:

  1. In includes/classes/class-core.php there's a method is_pjax() which is not used anywhere in this theme. The code should be removed, or add a pjax lib and use the code for whatever its intended for.
  1. You are specifying 'source' in tgm activation configs for your recommended plugins. You should review: http://tgmpluginactivation.com/configuration/#h-plugin-parameters. Plugins that are available in the wp.org repository should only specify the slug, the version numbers should be the minimum versions supported by your theme. This would help gain more trust for users installing those plugins as well as it would remove the label "External Source" from the activation page.
  1. In search.php you wrap get_search_query() in shadower_wp_kses() which is not necessary. The default value for $escaped on get_search_query() is set to true so esc_attr() is already used. You should remove your wrapping method.
  1. In includes/functions/internal/minimum-requirements.php you are hooking into admin_footer and wp_head then echoing out a script alert for failed version checks. The PHP code would still be running, so this doesn't do much for your users. You should really handle this on theme activation, and display the notice only in the wp admin - not on the frontend of the site where site visitors would see it. You should also be displaying this using the core provided way of adding alerts/notices to the dashboard. I would recommend something like this to resolve those issues:
if ( ! function_exists( 'shadower_minimum_requirements' ) && current_user_can( 'install_themes' ) ) {

	// Essentially the theme activation hook.
	// @see https://developer.wordpress.org/reference/hooks/after_switch_theme/
	add_action( 'after_switch_theme', 'shadower_minimum_requirements', 10, 2 );

	// Check minimum PHP version requirements.
	function shadower_minimum_requirements( $old_name, $old_theme ) {

		// Theme requires PHP 5.3+
		if ( version_compare( PHP_VERSION, '5.3.0' ) < 0 ) {

			// Add the admin notice to the WordPress dashboard.
			// @see https://developer.wordpress.org/reference/hooks/admin_notices/
			add_action( 'admin_notices', 'shadower_minimum_requirements_notice' );

			// Prints the markup for the admin notice.
			function shadower_minimum_requirements_notice() {
				?>
				<div class="error">
					<p>
						<?php printf(
							__( 'You\'re running WordPress on PHP version %s. Please contact your hosting company and updgrade PHP to 5.3 or above.', 'shadower' ),
							PHP_VERSION
						); ?>
					</p>
				</div>
				<?php
			}

			// Switch back to old theme if version check fails.
			// @see https://developer.wordpress.org/reference/functions/switch_theme/
			switch_theme( $old_theme->stylesheet );
			return false;
		}
	}
}
  1. Deprecated JS notice. In assets/js/script.js:852 you have:
if ( $.browser.msie && $.browser.version < 10 ) {
	$( '.animsition' ).addClass( 'bg-overlay-show' );		
} else {
	// ...animsition stuff
	$( '.animsition:not(.mobile)' ).animsition({

jQuery.browser is a deprecated method, and you shouldn't really rely on UA testing this way either. The browser key of the animistion config destroys animistion if the unsupported features are detected ( in your case for ie10 since you have the animation-duration and webkit-animation-duration css properties defined there). You are already loading Modernizr, so it would be best to just utilize that.

You could check it with something like:

if ( ! Modernizr.cssanimations ) {
	$( '.animsition' ).addClass( 'bg-overlay-show' );
} else {
	$( '.animsition:not(.mobile)' ).animsition({
	// ...animsition stuff

Or since this check already is done and classes are added to the html elem - you can reduce the cyclomatic complexity of it all with something like this:

$( '.no-cssanimations .animsition' ).addClass( 'bg-overlay-show' );
$( '.cssanimations .animsition:not(.mobile)' ).animsition({
	// ...animsition stuff
  1. The URL linked to in your "Go Pro" button is to https://uiux.cc/wp-theme-demo/shadower-landing-page/ which is a 404. This should be a valid URL.
  1. The theme mod custom_copyright has some of the default value sanitized, but allows any output since the sanitize_callback is just set to 'true'. You might consider using wp_kses_post as an alternative for example.
  1. From reading the license that the referenced flaticons "Basic Free" - my interpretation was that the license is not GPL compatible, but it does look like they offer a variety of different licenses for different icon packs as well. You should try to find an icon set that still fits your theme and is compatible with GPL: http://file000.flaticon.com/downloads/license/license.pdf
  1. PHP Parse error: syntax error, unexpected ';', expecting ',' or ')' in ./content-none.php on line 9

You might consider integrating a command like find . \( -name '*.php' \) -exec php -lf {} \; to help catch simple errors like this in the future.

  1. In author-bio.php L13 you have:
<?php shadower_wp_kses( __( 'Published by ', 'shadower' ) ); ?>

You should use esc_html_e() in place of shadower_wp_kses() here.

  1. In this same area, your translation string is seperated from any markup. If you plan to allow translators to add additional HTML to this translation ( I don't see why you would ), then you would want to use wp_kses and be more specific with what is actually allowed to pass through. It's also a good practice to not end a translation with a empty space. It would be best to use printf here 'Published by %s' and use get_the_author_meta(). This way you are explicitly giving the translator the appropriate context when they go to translate it, and not ending or beginning with empty spaces.

You can see an example of this at: https://developer.wordpress.org/reference/functions/the_author_meta/#comment-798

  1. In author-bio.php, you have this:
    $author_bio_avatar_size = apply_filters( 'shadower_author_bio_avatar_size', 100 );
    echo shadower_wp_kses( get_avatar( get_the_author_meta( 'user_email' ), $author_bio_avatar_size ) );
    

You don't need to add your shadower_wp_kses here, and you might want to cast $author_bio_avatar_size to an integer.

  1. In author-bio.php line 30 - you can remove esc_url() as this is already sanitized and trusted output.
  1. In author-bio.php line 31:
	<?php echo shadower_wp_kses( __( ' &rarr;', 'shadower' ) ); ?>

If that is your desired output, it does not need to be escaped, translated, or echoed.

  1. In author-bio.php line 15:

<a href="<?php echo esc_url( $user_url ); ?>" target="_blank">@<?php echo esc_html( $user_url ); ?></a>

Some Recommended Things:

  1. In assets/js/script.js:L51 you have the distance key added twice for your config object. You should only remove the one that isn't your desired configuration option.
  1. On lines 430 and 527 in n assets/js/script.js you have extra semicolons after your if/else statements. They aren't harming anything in this context, but it's generally a good idea to avoid this.
  1. Your sticky nav is not visible when a user is logged in. You should resolve this with some CSS like this:
.admin-bar .menu-wrapper.menu-scroll-fixed {
	top: 32px;
}
/* Admin bar becomes taller on smaller devices */
@media screen and ( max-width: 782px ) {
	.admin-bar .menu-wrapper.menu-scroll-fixed {
		top: 46px;
	}
}

Notes:

  1. Your navigation bar when you add a long menu to it doesn't wrap, so a user has to continue scrolling horizontally to see the menu items. The search box that slides out overlaps the menu text and becomes unusable if the menu reaches that length. This is not a great user experience, and should be given some consideration for those situations.
  1. For the custom_logo, you might consider allowing the user to adjust both width and height since the space there is pretty open and the user can always crop to make it fit as needed.
  1. It looks like you may have had various things integrated tightly into this theme in the past - perhaps used it as a base for other themes too. I would recommend going through your code and removing CSS, JS, and PHP code that is not actually in use for this theme to help speed up the review process.

I'd encourage you to evaluate your usage of shadower_wp_kses() throughout the theme, and when in doubt if a function needs to be escaped or not, look it up in the developer reference: https://developer.wordpress.org/reference/

Note: See TracTickets for help on using tickets.