WordPress.org

Make WordPress Themes

Opened 4 months ago

Closed 2 weeks ago

#45115 closed theme (live)

THEME: Compelling – 1.0.6

Reported by: stephencottontail Owned by: rinkuyadav999
Priority: new theme Keywords: theme-compelling
Cc: stephencottontail@…

Description

Compelling - 1.0.0

Your blog deserves the best. Bold. Beautiful. Compelling.

Theme URL -
Author URL - http://stephencottontail.com/

Trac Browser - https://themes.trac.wordpress.org/browser/compelling/1.0.0

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

History:

Ticket Summary Status Resolution Owner
#45115 THEME: Compelling – 1.0.6 closed live rinkuyadav999

(this ticket)


https://themes.svn.wordpress.org/compelling/1.0.0/screenshot.png
Theme Check Results:

  • RECOMMENDED: No reference to add_theme_support( "custom-header", $args ) was found in the theme. It is recommended that the theme implement this functionality if using an image for the header.
  • RECOMMENDED: No reference to add_theme_support( "custom-background", $args ) was found in the theme. If the theme uses background images or solid colors for the background, then it is recommended that the theme implement this functionality.
  • RECOMMENDED: Theme URI: is missing from your style.css header.

Change History (33)

#1 @themetracbot
4 months ago

  • Summary changed from THEME: Compelling – 1.0.0 to THEME: Compelling – 1.0.1

Compelling - 1.0.1

Your blog deserves the best. Bold. Beautiful. Compelling.

Theme URL -
Author URL - http://stephencottontail.com/

Trac Browser - https://themes.trac.wordpress.org/browser/compelling/1.0.1

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

Diff with previous version: https://themes.trac.wordpress.org/changeset?old_path=compelling/1.0.0&new_path=compelling/1.0.1

History:

Ticket Summary Status Resolution Owner
#45115 THEME: Compelling – 1.0.6 closed live rinkuyadav999

(this ticket)


https://themes.svn.wordpress.org/compelling/1.0.1/screenshot.png
Theme Check Results:

  • RECOMMENDED: No reference to add_theme_support( "custom-header", $args ) was found in the theme. It is recommended that the theme implement this functionality if using an image for the header.
  • RECOMMENDED: No reference to add_theme_support( "custom-background", $args ) was found in the theme. If the theme uses background images or solid colors for the background, then it is recommended that the theme implement this functionality.
  • RECOMMENDED: Theme URI: is missing from your style.css header.

#2 @themetracbot
3 months ago

  • Summary changed from THEME: Compelling – 1.0.1 to THEME: Compelling – 1.0.2

Compelling - 1.0.2

Your blog deserves the best. Bold. Beautiful. Compelling.

Theme URL -
Author URL - http://stephencottontail.com/

Trac Browser - https://themes.trac.wordpress.org/browser/compelling/1.0.2

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

Diff with previous version: https://themes.trac.wordpress.org/changeset?old_path=compelling/1.0.1&new_path=compelling/1.0.2

History:

Ticket Summary Status Resolution Owner
#45115 THEME: Compelling – 1.0.6 closed live rinkuyadav999

(this ticket)


https://themes.svn.wordpress.org/compelling/1.0.2/screenshot.png
Theme Check Results:

  • RECOMMENDED: No reference to add_theme_support( "custom-header", $args ) was found in the theme. It is recommended that the theme implement this functionality if using an image for the header.
  • RECOMMENDED: No reference to add_theme_support( "custom-background", $args ) was found in the theme. If the theme uses background images or solid colors for the background, then it is recommended that the theme implement this functionality.
  • RECOMMENDED: Theme URI: is missing from your style.css header.

#3 @themetracbot
2 months ago

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

#4 @AbdulWahab610
2 months ago

Hi @stephencottontail

I'm just assigned this theme to review. It is a formal message and let me know if you are active, Say Hello, and I will then start reviewing your theme.

If there is anything you want to change or update the version, Please do that and Reply.

Cheers!

#6 @AbdulWahab610
2 months ago

Hi, @stephencottontail

I have gone through your theme and found the issue, Please fix them and upload, I will review them again.

Your theme contains fatal errors which restrict the theme even form activating. https://prnt.sc/gprd62
If one error is fixed next one pops up! Please fix those fatal errors and then I will start a proper review.
Make sure to enable debugging mode in your WordPress environment.

Last edited 2 months ago by AbdulWahab610 (previous) (diff)

#7 @themetracbot
2 months ago

  • Summary changed from THEME: Compelling – 1.0.2 to THEME: Compelling – 1.0.3

Compelling - 1.0.3

Your blog deserves the best. Bold. Beautiful. Compelling.

Theme URL -
Author URL - http://stephencottontail.com/

Trac Browser - https://themes.trac.wordpress.org/browser/compelling/1.0.3

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

Diff with previous version: https://themes.trac.wordpress.org/changeset?old_path=compelling/1.0.2&new_path=compelling/1.0.3

History:

Ticket Summary Status Resolution Owner
#45115 THEME: Compelling – 1.0.6 closed live rinkuyadav999

(this ticket)


https://themes.svn.wordpress.org/compelling/1.0.3/screenshot.png
Theme Check Results:

  • RECOMMENDED: No reference to add_theme_support( "custom-header", $args ) was found in the theme. It is recommended that the theme implement this functionality if using an image for the header.
  • RECOMMENDED: No reference to add_theme_support( "custom-background", $args ) was found in the theme. If the theme uses background images or solid colors for the background, then it is recommended that the theme implement this functionality.
  • RECOMMENDED: Theme URI: is missing from your style.css header.

#8 @stephencottontail
2 months ago

The error should be fixed now, but I can't be certain because I was never able to actually get the same error to show up on my install. What version of WordPress and PHP are you running?

#9 @AbdulWahab610
8 weeks ago

WordPress 4.8.2
PHP 7.1.9
I'll check the updated theme and give feedback to you.

Last edited 8 weeks ago by AbdulWahab610 (previous) (diff)

#10 @AbdulWahab610
7 weeks ago

Hi @stephencottontail

I'll give you a feedback tonight.

#11 @AbdulWahab610
7 weeks ago

Hello @stephencottontail

Sorry for the delay. I have gone through your theme and found one issue, Please fix them and upload.

Required

  • No removing or modifying non-presentational hooks. Please check customizer.php # 18 ref

Thank you.

#12 @stephencottontail
7 weeks ago

Are you saying that I should have that section in the Customizer, even though the theme doesn't support a custom background or custom colors?

#13 @AbdulWahab610
7 weeks ago

No, but you can adjust the parameters to add_theme_support() to make them match what your theme does.

#14 @stephencottontail
7 weeks ago

I already don't add support for custom background or custom colors in add_theme_support(). Just so I'm absolutely clear on the requirements, are you saying that the Colors section of the Customizer needs to be present in my theme, even though that section would be completely non-functional?

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


7 weeks ago

#16 @thinkupthemes
7 weeks ago

Hello @stephencottontail

To answer your question, the color option will only show if your theme is adding functionality that uses the option. What you need to do is debug your theme to find out what exactly is causing the color option to display in the customizer. Then update your code to ensure it's not being output. You should not be removing it by adding $wp_customize->remove_section( 'colors' ); as you've done so here.

If you choose to keep the color option then it should work for it's intended purpose.

#17 @williampatton
7 weeks ago

@stephencottontail The customizer builds those sections programmatically based on what set of options are to be shown on a given page. If a section is empty it will not be added to the menu list with the other sections.

It may seem like unregistering the section is the best move here as you do not use it but what if a different plugin wants to use it? The plugin will load the settings into the section but, since themes fire later than plugins by default, the theme unregisters the entire section thus making the settings the plugin added completely inaccessible through customizer.

There is always a good reason behind one of the guidelines. If you are ever unsure of a reason behind a specific guideline then feel free to ping me in ticket or in slack (same username) and I'll be happy to help :)

#18 @themetracbot
7 weeks ago

  • Summary changed from THEME: Compelling – 1.0.3 to THEME: Compelling – 1.0.4

Compelling - 1.0.4

Your blog deserves the best. Bold. Beautiful. Compelling.

Theme URL -
Author URL - http://stephencottontail.com/

Trac Browser - https://themes.trac.wordpress.org/browser/compelling/1.0.4

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

Diff with previous version: https://themes.trac.wordpress.org/changeset?old_path=compelling/1.0.3&new_path=compelling/1.0.4

History:

Ticket Summary Status Resolution Owner
#45115 THEME: Compelling – 1.0.6 closed live rinkuyadav999

(this ticket)


https://themes.svn.wordpress.org/compelling/1.0.4/screenshot.png
Theme Check Results:

  • RECOMMENDED: No reference to add_theme_support( "custom-header", $args ) was found in the theme. It is recommended that the theme implement this functionality if using an image for the header.
  • RECOMMENDED: No reference to add_theme_support( "custom-background", $args ) was found in the theme. If the theme uses background images or solid colors for the background, then it is recommended that the theme implement this functionality.
  • RECOMMENDED: Theme URI: is missing from your style.css header.

#19 @AbdulWahab610
7 weeks ago

  • Status changed from reviewing to approved

Hi @stephencottontail

Thanks for your update. It looks fine now.

#20 @williampatton
7 weeks ago

Hi @stephencottontail,

When clearing out my logs this morning I noticed a few items from when I tested your theme yesterday. These 2 items will need looked at before theme

  • template-parts/content-page.php:15 - The $destination variable on this line will never be defined so it will throw a notice instead of putting a link in like it's supposed to.
    • The code in the content.php file shows a conditional where $destination is defined but that is not present in the content-page.php file.
    • Additionally I encourage you to do your escaping as late as possible (such as when you echo this value) so that it doesn't throw a notice to PHPCS. This would do no harm and also escape the return of get_attachment_link() (which is filtered immediately before return so it may not actually contain a valid escaped url).
  • inc/template-tags.php:76 - Overriding of globals is not allowed in theme. This code is not necessary as all it does is assign the current $comment object to the global. $comment is pulled from the global so both objects should be the same.

#21 @rinkuyadav999
4 weeks ago

  • Status changed from approved to reopened

Hi @stephencottontail

I will take final review.

Thanks

#22 @rinkuyadav999
4 weeks ago

  • Owner changed from AbdulWahab610 to rinkuyadav999
  • Status changed from reopened to reviewing

#23 @rinkuyadav999
4 weeks ago

Hi @stephencottontail

Please fix below issues:

compelling.js L54 remove unnecessary semicolon.

Notice: Undefined variable: destination in \template-parts\content-page.php on line 15. I think, you want to echo esc_url( get_permalink().

template-tags.php L38, Do not make comma to translation ready. Also it does not escape. Only escape database value and translation ready attributes of element. Check same and fix in all files.

template-tags.php L83, Pingback, it does not need to be escape. Just make it translation ready. Check same and fix in all files.

content.php L16 get_attachment_link, it need escape, esc url.

compelling/inc/template-tags.php L76 ERROR Overriding WordPress globals is prohibited

Theme tag: sticky-post, To use this tag, sticky post should appear different than normal posts. Or you can remove this tag from style.css.

functions.php L54, remove comment-list due to callback compelling_show_comment in wp_list_comments.

template-front-page.php L20, there is no need of wp_reset_postdata above loop. use it below loop so it can restores the $post global to the current post.

Thanks

#24 follow-up: @stephencottontail
4 weeks ago

template-tags.php L38, Do not make comma to translation ready. Also it does not escape. Only escape database value and translation ready attributes of element. Check same and fix in all files.

Twenty Seventeen does the same thing on line 65 of inc/template-tags.php. I was under the impression that you would want to translate the comma because other languages use different separators in that context.

functions.php L54, remove comment-list due to callback compelling_show_comment in wp_list_comments.

I don't understand why I would want to do this. If I removed comment-list, I would lose it as a styling hook, and I also don't understand why it matters that I use a callback.

#25 in reply to: ↑ 24 @rinkuyadav999
4 weeks ago

Replying to stephencottontail:

template-tags.php L38, Do not make comma to translation ready. Also it does not escape. Only escape database value and translation ready attributes of element. Check same and fix in all files.

Twenty Seventeen does the same thing on line 65 of inc/template-tags.php. I was under the impression that you would want to translate the comma because other languages use different separators in that context.

Okay you can make it translation ready but do not need escape.

functions.php L54, remove comment-list due to callback compelling_show_comment in wp_list_comments.

I don't understand why I would want to do this. If I removed comment-list, I would lose it as a styling hook, and I also don't understand why it matters that I use a callback.

Because callback is rendering comment lists instead core WP.

#26 @stephencottontail
4 weeks ago

Because callback is rendering comment lists instead core WP.

I guess I still don't understand why that matters. If you look at the callback function, it only supplies the <li> tag and I surround it with <ol class="comment-list">...</ol> so I can use it for styling purposes and so the HTML will be correct.

#27 @rinkuyadav999
4 weeks ago

@stephencottontail let wait for @rabmalin opinion for ticket:45115#comment:26 .

#28 @rabmalin
4 weeks ago

  • REQUIRED: You should either remove comment-list from add_theme_support('html5') and render comment list yourself using callback in wp_list_comments() OR keep comment-list in add_theme_support('html5) and remove 'callback' => 'compelling_show_comment' from wp_list_comments()
  • RECOMMENDED: Use https://developer.wordpress.org/reference/functions/the_comments_navigation/ for comment navigation.
  • REQUIRED: In readme file, please declare your theme copyright in following format.
    Fred WordPress Theme, Copyright 2012 Joe Smith
    Fred is distributed under the terms of the GNU GPL
    

Sniffer Isses

Note: Errors need to be fixed and Warnings are things that need to be checked manually.

FILE: /var/www/review.dev/public_html/wp-content/themes/compelling/inc/template-tags.php
----------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------------------------
 76 | ERROR | Overriding WordPress globals is prohibited
----------------------------------------------------------------------------------------

#29 @stephencottontail
4 weeks ago

Oh, okay, I understand now; I thought you meant I needed to remove comment-list from the <ol> tag in comments.php.

#30 @themetracbot
3 weeks ago

  • Summary changed from THEME: Compelling – 1.0.4 to THEME: Compelling – 1.0.5

Compelling - 1.0.5

Your blog deserves the best. Bold. Beautiful. Compelling.

Theme URL -
Author URL - http://stephencottontail.com/

Trac Browser - https://themes.trac.wordpress.org/browser/compelling/1.0.5

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

Diff with previous version: https://themes.trac.wordpress.org/changeset?old_path=compelling/1.0.4&new_path=compelling/1.0.5

History:

Ticket Summary Status Resolution Owner
#45115 THEME: Compelling – 1.0.6 closed live rinkuyadav999

(this ticket)


https://themes.svn.wordpress.org/compelling/1.0.5/screenshot.png
Theme Check Results:

  • RECOMMENDED: No reference to add_theme_support( "custom-header", $args ) was found in the theme. It is recommended that the theme implement this functionality if using an image for the header.
  • RECOMMENDED: No reference to add_theme_support( "custom-background", $args ) was found in the theme. If the theme uses background images or solid colors for the background, then it is recommended that the theme implement this functionality.
  • RECOMMENDED: Theme URI: is missing from your style.css header.

#31 @rinkuyadav999
3 weeks ago

Hi @stephencottontail

Please use wp_reset_postdata() in template-front-page.php.

Thanks

#32 @themetracbot
3 weeks ago

  • Summary changed from THEME: Compelling – 1.0.5 to THEME: Compelling – 1.0.6

Compelling - 1.0.6

Your blog deserves the best. Bold. Beautiful. Compelling.

Theme URL -
Author URL - http://stephencottontail.com/

Trac Browser - https://themes.trac.wordpress.org/browser/compelling/1.0.6

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

Diff with previous version: https://themes.trac.wordpress.org/changeset?old_path=compelling/1.0.5&new_path=compelling/1.0.6

History:

Ticket Summary Status Resolution Owner
#45115 THEME: Compelling – 1.0.6 closed live rinkuyadav999

(this ticket)


https://themes.svn.wordpress.org/compelling/1.0.6/screenshot.png
Theme Check Results:

  • RECOMMENDED: No reference to add_theme_support( "custom-header", $args ) was found in the theme. It is recommended that the theme implement this functionality if using an image for the header.
  • RECOMMENDED: No reference to add_theme_support( "custom-background", $args ) was found in the theme. If the theme uses background images or solid colors for the background, then it is recommended that the theme implement this functionality.
  • RECOMMENDED: Theme URI: is missing from your style.css header.

#33 @rinkuyadav999
2 weeks ago

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

Looks good to me. Setting live.

Note: See TracTickets for help on using tickets.