WordPress.org

Make WordPress Themes

Opened 2 years ago

Closed 20 months ago

#26301 closed theme (live)

THEME: Kicoe – 1.8

Reported by: kicoe Owned by: kevinhaig
Priority: previously reviewed Keywords: theme-kicoe
Cc: mr.kicoe@…

Description

Kicoe - 1.5

Kicoe is a fully responsive personal template : Colorful blog, curriculum vitae, simple content pages and personnal branding. It just keeps it simple and efficient.

Theme URL - http://wpthemes.patrickroux.fr/kicoe
Author URL - http://www.patrickoux.fr

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

Diff with previous version: https://themes.trac.wordpress.org/changeset?old_path=kicoe/1.4&new_path=kicoe/1.5

History:

Ticket Summary Status Resolution Owner
#22632 THEME: Kicoe – 1.4 closed not-approved poena
#26301 THEME: Kicoe – 1.8 closed live kevinhaig

(this ticket)


https://themes.svn.wordpress.org/kicoe/1.5/screenshot.png

Attachments (7)

kicoe-error.jpg (227.2 KB) - added by andtrev 2 years ago.
screenshot
kicoe-image-overflow.jpg (45.8 KB) - added by andtrev 2 years ago.
Image overflow
kicoe-newer-comments.jpg (41.4 KB) - added by andtrev 2 years ago.
Comment pagination
kicoe-many-categories.jpg (56.1 KB) - added by andtrev 2 years ago.
Many category issue
kicoe-widget-notices.jpg (137.6 KB) - added by andtrev 23 months ago.
Widget PHP notices
kicoe-two-color-pickers.jpg (75.5 KB) - added by andtrev 23 months ago.
Side by side color picker buttons
kicoe-many-tags.jpg (102.0 KB) - added by andtrev 23 months ago.
Many tags

Download all attachments as: .zip

Change History (65)

#1 follow-up: @themetracbot
2 years ago

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

#2 in reply to: ↑ 1 @kicoe
2 years ago

Replying to themetracbot:
Thank you for reviewing my theme ... !

#3 follow-ups: @karmatosed
2 years ago

I am sorry this review is taking so long. Sometimes people are unable to carry on the review, this may have happened this time. As a result, I am going to add this to the urgent review queue. This may mean it still takes time, but should be of more priority. Thanks for your patience.

#4 in reply to: ↑ 3 @kicoe
2 years ago

Replying to karmatosed:

I am sorry this review is taking so long. Sometimes people are unable to carry on the review, this may have happened this time. As a result, I am going to add this to the urgent review queue. This may mean it still takes time, but should be of more priority. Thanks for your patience.

Thank you for your concern, I just challenge the reviewer to help this theme not to celebrate its first anniversary pending review :-)

#5 @Otto42
2 years ago

  • Owner ilyabd deleted

#6 in reply to: ↑ 3 @kicoe
2 years ago

Replying to karmatosed:

I am sorry this review is taking so long. Sometimes people are unable to carry on the review, this may have happened this time. As a result, I am going to add this to the urgent review queue. This may mean it still takes time, but should be of more priority. Thanks for your patience.

Hi,
Would it be possible to have some insight, answer, any kind of reaction? To be honest after 10 months of waiting from the first upload of my theme and 3 months for this new version, I'm beginning to wonder if this has not been forgotten somehow...?
Thank you for the followup...

#7 follow-up: @Otto42
2 years ago

@kicoe We recently implemented a change to help get these "forgotten" themes back on track. When I deleted the reviewer 11 days ago, this put it back in the New theme queue, and the older ones are top of the list. So reviews for these old tickets will be processed first.

However, there were a lot of drive-by "reviewers" who took themes out of the queue and then never followed up on them, thus putting them in this sort of purgatory state. We're trying to fix that now by making these themes the current top priority, but again, quite a large number. It's being worked through, but the old tickets are currently top of the list.

#8 in reply to: ↑ 7 @kicoe
2 years ago

Replying to Otto42:

@kicoe We recently implemented a change to help get these "forgotten" themes back on track. When I deleted the reviewer 11 days ago, this put it back in the New theme queue, and the older ones are top of the list. So reviews for these old tickets will be processed first.

However, there were a lot of drive-by "reviewers" who took themes out of the queue and then never followed up on them, thus putting them in this sort of purgatory state. We're trying to fix that now by making these themes the current top priority, but again, quite a large number. It's being worked through, but the old tickets are currently top of the list.

Thank you for this information.

But I wonder... My theme was first uploaded 10 months ago, I just reopened a new ticket because the ghost reviewer aked for it... So I just looked up the 479 not closed themes, with a 10 months history mine should be 3rd in line, would that assumption be correct?

#9 follow-up: @Otto42
2 years ago

No, this ticket was opened 3 months ago. There's 31 themes ahead of you.

See, you didn't open a new ticket for the "ghost reviewer". Your theme was reviewed 3 or 4 times over here: https://themes.trac.wordpress.org/ticket/22632

Since you didn't respond in that review, eventually that ticket was closed. This new one is only 3 months old, from the time you uploaded the latest version.

#10 in reply to: ↑ 9 @kicoe
2 years ago

Replying to Otto42:

No, this ticket was opened 3 months ago. There's 31 themes ahead of you.

See, you didn't open a new ticket for the "ghost reviewer". Your theme was reviewed 3 or 4 times over here: https://themes.trac.wordpress.org/ticket/22632

Since you didn't respond in that review, eventually that ticket was closed. This new one is only 3 months old, from the time you uploaded the latest version.

Well, to be honnest I would have appreciated to be given a little bit more time to answer something before the ticket was closed... Anyway, I understand you have a lot of theme to take care about and I thank you and your teammates for that, I'll wait my turn. I just hope it will come before 3 months ;) !

#11 @themetracbot
2 years ago

  • Owner set to andtrev

@andtrev
2 years ago

screenshot

#12 follow-ups: @andtrev
2 years ago

Hi @kicoe, I've been assigned to review your theme, I understand it's taken awhile, I apologize for that, but we'll do our best to get this theme approved. Here are my notes:

Required

  1. Line 15 of custom-widgets.php - parent::WP_Widget(false, $name = __('Kicoe Profile', 'wp_widget_plugin') ); is deprecated - https://make.wordpress.org/core/2015/07/02/deprecating-php4-style-constructors-in-wordpress-4-3/. You must change that to something like this:
    $widget_ops = array('classname' => 'widget_kicoe_profile', 'description' => __( 'Description of widget.' , 'kicoe' ) );
    parent::__construct('kicoe_profile', __('Kicoe Profile', 'kicoe'), $widget_ops);
    
  2. comments.php has a text-domain usage of 'framework' in it, please change to the correct text-domain for the theme
  3. Undefined variable $theme_data in /inc/footer/footer.php Why not simply have /inc/footer/footer.php in your footer.php?
  4. Undefined variable $post_id in content-single.php
  5. There's some french language in some parts, search.php, custom-widgets.php - change to all english like the rest of the theme
  6. After activating the theme and navigating to the front page I see a broken layout for dates, this needs to be fixed - I've attached a screenshot so you can see what I see above this post, this was on WordPress 4.3.1 - the background is fixed and so looks funny in the screenshot, but is fine on the live site

Your screenshot is 600x450, it's recommended it be at least 880x660 with 1200x900 preferred

Last edited 2 years ago by andtrev (previous) (diff)

#13 in reply to: ↑ 12 @kicoe
2 years ago

Hi andtrev,
Thank you for your review. Just to let you know I'll post a new version within 2-3 days.

Replying to andtrev:

Hi @kicoe, I've been assigned to review your theme, I understand it's taken awhile, I apologize for that, but we'll do our best to get this theme approved. Here are my notes:

Required

  1. Line 15 of custom-widgets.php - parent::WP_Widget(false, $name = __('Kicoe Profile', 'wp_widget_plugin') ); is deprecated - https://make.wordpress.org/core/2015/07/02/deprecating-php4-style-constructors-in-wordpress-4-3/. You must change that to something like this:
    $widget_ops = array('classname' => 'widget_kicoe_profile', 'description' => __( 'Description of widget.' , 'kicoe' ) );
    parent::__construct('kicoe_profile', __('Kicoe Profile', 'kicoe'), $widget_ops);
    
  2. comments.php has a text-domain usage of 'framework' in it, please change to the correct text-domain for the theme
  3. Undefined variable $theme_data in /inc/footer/footer.php Why not simply have /inc/footer/footer.php in your footer.php?
  4. Undefined variable $post_id in content-single.php
  5. There's some french language in some parts, search.php, custom-widgets.php - change to all english like the rest of the theme
  6. After activating the theme and navigating to the front page I see a broken layout for dates, this needs to be fixed - I've attached a screenshot so you can see what I see above this post, this was on WordPress 4.3.1 - the background is fixed and so looks funny in the screenshot, but is fine on the live site

Your screenshot is 600x450, it's recommended it be at least 880x660 with 1200x900 preferred

#14 in reply to: ↑ 12 @kicoe
2 years ago

Could you tell me what date format you used?
On my demo theme I have the default date format and I fail to see any issue...

Replying to andtrev:

Hi @kicoe, I've been assigned to review your theme, I understand it's taken awhile, I apologize for that, but we'll do our best to get this theme approved. Here are my notes:

Required

  1. Line 15 of custom-widgets.php - parent::WP_Widget(false, $name = __('Kicoe Profile', 'wp_widget_plugin') ); is deprecated - https://make.wordpress.org/core/2015/07/02/deprecating-php4-style-constructors-in-wordpress-4-3/. You must change that to something like this:
    $widget_ops = array('classname' => 'widget_kicoe_profile', 'description' => __( 'Description of widget.' , 'kicoe' ) );
    parent::__construct('kicoe_profile', __('Kicoe Profile', 'kicoe'), $widget_ops);
    
  2. comments.php has a text-domain usage of 'framework' in it, please change to the correct text-domain for the theme
  3. Undefined variable $theme_data in /inc/footer/footer.php Why not simply have /inc/footer/footer.php in your footer.php?
  4. Undefined variable $post_id in content-single.php
  5. There's some french language in some parts, search.php, custom-widgets.php - change to all english like the rest of the theme
  6. After activating the theme and navigating to the front page I see a broken layout for dates, this needs to be fixed - I've attached a screenshot so you can see what I see above this post, this was on WordPress 4.3.1 - the background is fixed and so looks funny in the screenshot, but is fine on the live site

Your screenshot is 600x450, it's recommended it be at least 880x660 with 1200x900 preferred

#15 @themetracbot
2 years ago

  • Summary changed from THEME: Kicoe – 1.5 to THEME: Kicoe – 1.6

Kicoe - 1.6

Kicoe is a fully responsive personal template : Colorful blog, curriculum vitae, simple content pages and personnal branding. It just keeps it simple and efficient.

Theme URL - http://wpthemes.patrickroux.fr/kicoe
Author URL - http://www.patrickoux.fr

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

Diff with previous version: https://themes.trac.wordpress.org/changeset?old_path=kicoe/1.5&new_path=kicoe/1.6

History:

Ticket Summary Status Resolution Owner
#22632 THEME: Kicoe – 1.4 closed not-approved poena
#26301 THEME: Kicoe – 1.8 closed live kevinhaig

(this ticket)


https://themes.svn.wordpress.org/kicoe/1.6/screenshot.png

#16 in reply to: ↑ 12 @kicoe
2 years ago

For the date problem, I tried to use the_time instead of the_date, and added some css for non thumbnail posts.
Regarding the rest of your recommandation, feel free to tell me if I got it right :)

Thank you again

Replying to andtrev:

Hi @kicoe, I've been assigned to review your theme, I understand it's taken awhile, I apologize for that, but we'll do our best to get this theme approved. Here are my notes:

Required

  1. Line 15 of custom-widgets.php - parent::WP_Widget(false, $name = __('Kicoe Profile', 'wp_widget_plugin') ); is deprecated - https://make.wordpress.org/core/2015/07/02/deprecating-php4-style-constructors-in-wordpress-4-3/. You must change that to something like this:
    $widget_ops = array('classname' => 'widget_kicoe_profile', 'description' => __( 'Description of widget.' , 'kicoe' ) );
    parent::__construct('kicoe_profile', __('Kicoe Profile', 'kicoe'), $widget_ops);
    
  2. comments.php has a text-domain usage of 'framework' in it, please change to the correct text-domain for the theme
  3. Undefined variable $theme_data in /inc/footer/footer.php Why not simply have /inc/footer/footer.php in your footer.php?
  4. Undefined variable $post_id in content-single.php
  5. There's some french language in some parts, search.php, custom-widgets.php - change to all english like the rest of the theme
  6. After activating the theme and navigating to the front page I see a broken layout for dates, this needs to be fixed - I've attached a screenshot so you can see what I see above this post, this was on WordPress 4.3.1 - the background is fixed and so looks funny in the screenshot, but is fine on the live site

Your screenshot is 600x450, it's recommended it be at least 880x660 with 1200x900 preferred

#17 follow-up: @andtrev
2 years ago

Checking in to let you know my status, I'll be reviewing again later tonight after I get off work and will post my findings, right off the bat I see a few things though:

  • __MACOSX folder needs to be removed
  • There's more french language in the comments.php file
  • Remove the commented line //parent::WP_Widget(false, $name = __('Kicoe Profile', 'wp_widget_plugin') ); in custom-widgets.php as it's maybe confusing to why it's there
  • Remove the rtl.css file as it has nothing in it
  • Why two css files? /assets/css/main.css and style.css should really be combined, this will help in two ways, one it will be easier for child theme authors to find all the necessary styles, and it will allow child theme authors to override those styles more easily. Right now you're enqueuing style.css before main.css, which makes it more difficult to override the styles in main.css
  • The code for moving around the wpautop and shortcode_unautop at the bottom of functions.php needs to be removed, as this is plugin territory and themes are not allowed to alter/change user content
    //move wpautop filter to AFTER shortcode is processed
    add_filter( 'the_content', 'wpautop' , 99);
    add_filter( 'the_content', 'shortcode_unautop',100 );
    
  • Also remove this unused code from functions.php
    /**
     * Customizer additions.
     * require get_template_directory() . '/inc/customizer.php';
     */
    

I will let you know if the date layout fix worked tonight and check through the entire layout to be sure it's all working.

#18 in reply to: ↑ 17 @kicoe
2 years ago

Thank you for this review,

I'm uploading version 1.6.1. To answer your question about main.css, this is simply to separate the reset and default styles from what should be customed to basically alter the design. If someone wants to create a child theme, I don't see any difficulty in the fact of editing a second css file. Besides and more importantly here, isn't that compliant with wp themes requirements? I have not seen any recommandations about not doing that, I guess it's a mater of preferences...

Regarding the rest of your remarks, I think I covered them all but I am not sure about some french words I could have not seen... Thank you again.

Replying to andtrev:

Checking in to let you know my status, I'll be reviewing again later tonight after I get off work and will post my findings, right off the bat I see a few things though:

  • __MACOSX folder needs to be removed
  • There's more french language in the comments.php file
  • Remove the commented line //parent::WP_Widget(false, $name = __('Kicoe Profile', 'wp_widget_plugin') ); in custom-widgets.php as it's maybe confusing to why it's there
  • Remove the rtl.css file as it has nothing in it
  • Why two css files? /assets/css/main.css and style.css should really be combined, this will help in two ways, one it will be easier for child theme authors to find all the necessary styles, and it will allow child theme authors to override those styles more easily. Right now you're enqueuing style.css before main.css, which makes it more difficult to override the styles in main.css
  • The code for moving around the wpautop and shortcode_unautop at the bottom of functions.php needs to be removed, as this is plugin territory and themes are not allowed to alter/change user content
    //move wpautop filter to AFTER shortcode is processed
    add_filter( 'the_content', 'wpautop' , 99);
    add_filter( 'the_content', 'shortcode_unautop',100 );
    
  • Also remove this unused code from functions.php
    /**
     * Customizer additions.
     * require get_template_directory() . '/inc/customizer.php';
     */
    

I will let you know if the date layout fix worked tonight and check through the entire layout to be sure it's all working.

#19 @themetracbot
2 years ago

  • Summary changed from THEME: Kicoe – 1.6 to THEME: Kicoe – 1.6.1

Kicoe - 1.6.1

Kicoe is a fully responsive personal template : Colorful blog, curriculum vitae, simple content pages and personnal branding. It just keeps it simple and efficient.

Theme URL - http://wpthemes.patrickroux.fr/kicoe
Author URL - http://www.patrickoux.fr

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

Diff with previous version: https://themes.trac.wordpress.org/changeset?old_path=kicoe/1.6&new_path=kicoe/1.6.1

History:

Ticket Summary Status Resolution Owner
#22632 THEME: Kicoe – 1.4 closed not-approved poena
#26301 THEME: Kicoe – 1.8 closed live kevinhaig

(this ticket)


https://themes.svn.wordpress.org/kicoe/1.6.1/screenshot.png

#20 follow-up: @andtrev
2 years ago

Hi Kicoe, here are my points this time around:

Looks like your date fix worked for me.

As for the conversation about the style.css and main.css, your logic is correct, but your implementation isn't. Child theming should be easy, but the way you've done it makes it more difficult. Also Bootstrap already includes the normalize reset. Because the only css file I can override in a child theme is the style.css it doesn't make sense for this to only be a reset, why would I want to override just that? And because that sheet is linked to before the main.css stylesheet I can't easily override those rules without using !important a lot. It adds another layer of complexity to something that should be very simple. If you ditch the main.css file altogether, and merge it with style.css, getting rid of the normalize reset in there and enqueue after bootstrap it makes much more sense.

  • Remove the __MACOSX folder
  • Remove this unused code from functions.php
    /**
     * Customizer additions.
     * require get_template_directory() . '/inc/customizer.php';
     */
    
  • Remove the $name parameter (which is unused) from the loadfooter function in /inc/footer/footer.php
  • May as well remove the README.md file, as that's all included in the readme.txt file
  • A non aligned image will overflow the post container (see attached image below), you need some css like img { max-width: 100%; height: auto; } to prevent this from happening
  • Comment pagination is covered by comment boxes at top of comment section (see attached image below) (to test this change the "Break comments into pages" setting in Settings -> Discussion to something low enough to cause pagination)
  • When there are a lot of categories on a post, it pushes down the content without displaying the category list, which leaves an lot of white space (see attached image below)
  • French language on line 48 of content.php and lines 38,39 of /inc/template-tags.php
  • You need to add backward compatibility for the title-tag theme support instead of just hard coding in the title tag in header.php - see this post for reference https://make.wordpress.org/core/2014/10/29/title-tags-in-4-1/

Ok, it's the holidays here so I'm going to have to stop for the moment, I don't think there should be much else after this, but I should give it another once over just to be sure. It's a great looking theme.

@andtrev
2 years ago

Image overflow

@andtrev
2 years ago

Comment pagination

@andtrev
2 years ago

Many category issue

#21 @themetracbot
2 years ago

  • Summary changed from THEME: Kicoe – 1.6.1 to THEME: Kicoe – 1.6.2

Kicoe - 1.6.2

Kicoe is a fully responsive personal template : Colorful blog, curriculum vitae, simple content pages and personnal branding. It just keeps it simple and efficient.

Theme URL - http://wpthemes.patrickroux.fr/kicoe
Author URL - http://www.patrickoux.fr

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

Diff with previous version: https://themes.trac.wordpress.org/changeset?old_path=kicoe/1.6.1&new_path=kicoe/1.6.2

History:

Ticket Summary Status Resolution Owner
#22632 THEME: Kicoe – 1.4 closed not-approved poena
#26301 THEME: Kicoe – 1.8 closed live kevinhaig

(this ticket)


https://themes.svn.wordpress.org/kicoe/1.6.2/screenshot.png

#22 in reply to: ↑ 20 @kicoe
2 years ago

Replying to andtrev:

Hi Kicoe, here are my points this time around:

Looks like your date fix worked for me.

As for the conversation about the style.css and main.css, your logic is correct, but your implementation isn't. Child theming should be easy, but the way you've done it makes it more difficult. Also Bootstrap already includes the normalize reset. Because the only css file I can override in a child theme is the style.css it doesn't make sense for this to only be a reset, why would I want to override just that? And because that sheet is linked to before the main.css stylesheet I can't easily override those rules without using !important a lot. It adds another layer of complexity to something that should be very simple. If you ditch the main.css file altogether, and merge it with style.css, getting rid of the normalize reset in there and enqueue after bootstrap it makes much more sense.

I merged the two CSS into one. There may be some redundant styles, but when I tried to get rid of them too many side effects... I plan on working on that for V2, would it be ok?

  • Remove the __MACOSX folder

This is a pain in the * with MACs... I tried to use ZipCleaner this time...

  • Remove this unused code from functions.php
    /**
     * Customizer additions.
     * require get_template_directory() . '/inc/customizer.php';
     */
    

Did not find this particular code, but found another one that I removed

  • Remove the $name parameter (which is unused) from the loadfooter function in /inc/footer/footer.php

Done

  • May as well remove the README.md file, as that's all included in the readme.txt file

Done

  • A non aligned image will overflow the post container (see attached image below), you need some css like img { max-width: 100%; height: auto; } to prevent this from happening

Done, thanks for the tip

  • Comment pagination is covered by comment boxes at top of comment section (see attached image below) (to test this change the "Break comments into pages" setting in Settings -> Discussion to something low enough to cause pagination)

Done

  • When there are a lot of categories on a post, it pushes down the content without displaying the category list, which leaves an lot of white space (see attached image below)

I found a way to tweak that : I only display the first category, but on mouse over and with a line of CSS I display the rest of it below. For the single article, I simply display the first on top, and the rest on the bottom below the tags.

  • French language on line 48 of content.php and lines 38,39 of /inc/template-tags.php

Désolé pour ça :)

Done, thank you for the tip

Ok, it's the holidays here so I'm going to have to stop for the moment, I don't think there should be much else after this, but I should give it another once over just to be sure. It's a great looking theme.

Thank you for your hard work, this review was thorough and I really appreciate it, just learned a few things in the process and I love that :) Happy thanks giving and I'm grateful for your compliment :)

#23 follow-up: @andtrev
23 months ago

Looks like the changes are good. Here's a few other things I've noticed:

  • Line 72 of content.php has french language in it
  • You need to remove the title tag from header.php as you add it now in functions.php
    <title><?php wp_title( '|', true, 'right' ); ?></title>
    
  • Your custom widget should have defaults set for the options, as of now it causes notices for undefined variables when added to the widgets in the admin area or in customizer - see attached image below
  • Defaults for the widget colors being set on addition to widget area in admin would help with figuring out which one to change for the end user
  • Widget has two color picker buttons side by side when first loaded - see attached image below
  • The widget form has french language in it
  • Widget "remembers" images after being dragged out of widget area and then a new instance dragged in
  • Line 16 of custom-widgets.php needs to have a real description and not the placeholder
  • The Couleur de police du pied section of the widget keeps popping open with the color picker even if not selected - such as after saving the widget state
  • You define a footer widget area, but I don't see anyplace that it outputs on the front end, am I missing it? If not then remove it from functions.php line 114
  • Not really a deal breaker, but just thought I'd let you know there's a lot of space between lines for tags, not sure if that was intentional or not - see image attached below
Last edited 23 months ago by andtrev (previous) (diff)

@andtrev
23 months ago

Widget PHP notices

@andtrev
23 months ago

Side by side color picker buttons

@andtrev
23 months ago

Many tags

#24 in reply to: ↑ 23 ; follow-up: @kicoe
23 months ago

Thanks a lot for this review.

I'm currently having difficulties regarding the double color widget thingy... In fact, my research shows that it is a known bug of wpColorPicker, would you agree? I could use some help on this matter, because I cannot find any working solution except using a third party color picker...
https://core.trac.wordpress.org/ticket/25809

Besides, I fail to see what you mean by "The Couleur de police du pied section of the widget keeps popping open with the color picker even if not selected - such as after saving the widget state" ?

Replying to andtrev:

Looks like the changes are good. Here's a few other things I've noticed:

  • Line 72 of content.php has french language in it
  • You need to remove the title tag from header.php as you add it now in functions.php
    <title><?php wp_title( '|', true, 'right' ); ?></title>
    
  • Your custom widget should have defaults set for the options, as of now it causes notices for undefined variables when added to the widgets in the admin area or in customizer - see attached image below
  • Defaults for the widget colors being set on addition to widget area in admin would help with figuring out which one to change for the end user
  • Widget has two color picker buttons side by side when first loaded - see attached image below
  • The widget form has french language in it
  • Widget "remembers" images after being dragged out of widget area and then a new instance dragged in
  • Line 16 of custom-widgets.php needs to have a real description and not the placeholder
  • The Couleur de police du pied section of the widget keeps popping open with the color picker even if not selected - such as after saving the widget state
  • You define a footer widget area, but I don't see anyplace that it outputs on the front end, am I missing it? If not then remove it from functions.php line 114
  • Not really a deal breaker, but just thought I'd let you know there's a lot of space between lines for tags, not sure if that was intentional or not - see image attached below

#25 @andtrev
23 months ago

I'll look this over again tonight, but the answer in that ticket should work to solve your issue. The code they linked to shows you how to implement it - https://core.trac.wordpress.org/attachment/ticket/25809/color-picker-widget.php

#26 in reply to: ↑ 24 @kicoe
23 months ago

Replying to andtrev:

  • Line 72 of content.php has french language in it

Ok, fixed

  • You need to remove the title tag from header.php as you add it now in functions.php
    <title><?php wp_title( '|', true, 'right' ); ?></title>
    

Ok, fixed

  • Your custom widget should have defaults set for the options, as of now it causes notices for undefined variables when added to the widgets in the admin area or in customizer - see attached image below

I added control ops to the constructor to fix this, but I initially failed to reproduce the error you encountered. Thus I cannot say for sure that this issue is resolved...

  • Defaults for the widget colors being set on addition to widget area in admin would help with figuring out which one to change for the end user

Ok, Done

  • Widget has two color picker buttons side by side when first loaded - see attached image below

Ok, fixed, thank you for your help on this matter

  • The widget form has french language in it

Ok, fixed

  • Widget "remembers" images after being dragged out of widget area and then a new instance dragged in

I modified the js to only trigger the media thingy on widget-added and widget-updated events. But If you try and remove the widget after reloading the page, the picture is removed. There is a way to erase the fields directly after removing the widget, but the only way I found to do that is to get rid of the little preview of the picture. I could just have a text field with the path of the picture, but I figured it would be best for the user to have the preview, no?

  • Line 16 of custom-widgets.php needs to have a real description and not the placeholder

Ok, fixed

  • The Couleur de police du pied section of the widget keeps popping open with the color picker even if not selected - such as after saving the widget state

Should be fixed with the correction I made regarding the double color picker

  • You define a footer widget area, but I don't see anyplace that it outputs on the front end, am I missing it? If not then remove it from functions.php line 114

That's correct, thanks. Fixed.

  • Not really a deal breaker, but just thought I'd let you know there's a lot of space between lines for tags, not sure if that was intentional or not - see image attached below

I'd love to say it was intentional and not at all something I missed, but let's face it.. THank you, it's fixed !

#27 @themetracbot
23 months ago

  • Summary changed from THEME: Kicoe – 1.6.2 to THEME: Kicoe – 1.6.3

Kicoe - 1.6.3

Kicoe is a fully responsive personal template : Colorful blog, curriculum vitae, simple content pages and personnal branding. It just keeps it simple and efficient.

Theme URL - http://wpthemes.patrickroux.fr/kicoe
Author URL - http://www.patrickoux.fr

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

Diff with previous version: https://themes.trac.wordpress.org/changeset?old_path=kicoe/1.6.2&new_path=kicoe/1.6.3

History:

Ticket Summary Status Resolution Owner
#22632 THEME: Kicoe – 1.4 closed not-approved poena
#26301 THEME: Kicoe – 1.8 closed live kevinhaig

(this ticket)


https://themes.svn.wordpress.org/kicoe/1.6.3/screenshot.png

#28 follow-up: @andtrev
23 months ago

All your fixes seem to have worked, here are a few more things I noticed this time around, not very much at least :)

  • Your "Read More" translation on archive/blog posts pages is "Read next" which doesn't make sense in the context it's displayed, as the intent is to read more, it sounds like I'm going to the next post or something. This is in the lower right of the post boxes.
  • If no banner image is selected, but a picture image is selected then the profile widget doesn't display correctly, it's too high up on the page and is covered by the header.
  • Why did you include the easy image uploader plugin in the inc folder? It should be removed.
  • There's currently no option to remove the image selections from the widget form.

I'd take a look at this codex example of how to use wp.media and implement something similar to this instead for image selection - http://codex.wordpress.org/Javascript_Reference/wp.media

Also be sure your included French translation is complete, I haven't looked at it to see if everything is translated or not, but would be good to check.

#29 in reply to: ↑ 28 @kicoe
23 months ago

Replying to andtrev:

All your fixes seem to have worked, here are a few more things I noticed this time around, not very much at least :)

  • Your "Read More" translation on archive/blog posts pages is "Read next" which doesn't make sense in the context it's displayed, as the intent is to read more, it sounds like I'm going to the next post or something. This is in the lower right of the post boxes.

Ok, done

  • If no banner image is selected, but a picture image is selected then the profile widget doesn't display correctly, it's too high up on the page and is covered by the header.

Fixed

  • Why did you include the easy image uploader plugin in the inc folder? It should be removed.

Removed

  • There's currently no option to remove the image selections from the widget form.

Added

Also be sure your included French translation is complete, I haven't looked at it to see if everything is translated or not, but would be good to check.

Checked !

#30 @themetracbot
23 months ago

  • Summary changed from THEME: Kicoe – 1.6.3 to THEME: Kicoe – 1.7

Kicoe - 1.7

Kicoe is a fully responsive personal template : Colorful blog, curriculum vitae, simple content pages and personnal branding. It just keeps it simple and efficient.

Theme URL - http://wpthemes.patrickroux.fr/kicoe
Author URL - http://www.patrickoux.fr

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

Diff with previous version: https://themes.trac.wordpress.org/changeset?old_path=kicoe/1.6.3&new_path=kicoe/1.7

History:

Ticket Summary Status Resolution Owner
#22632 THEME: Kicoe – 1.4 closed not-approved poena
#26301 THEME: Kicoe – 1.8 closed live kevinhaig

(this ticket)


https://themes.svn.wordpress.org/kicoe/1.7/screenshot.png

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


22 months ago

#32 follow-ups: @andtrev
22 months ago

Looks ok to me, one last thing is you need to sanitize and escape the data in the widget. Looks like you escaped output in the widget function, but not in the form function, and you should probably use WordPress sanitizing function instead of strip_tags in the update function This is a good article on doing that - http://code.tutsplus.com/articles/data-sanitization-and-validation-with-wordpress--wp-25536

After those fixes I can approve it and then they're telling me it'll take about a week for an admin to look it over and set it live, or let us know if something isn't right.

#33 in reply to: ↑ 32 @kicoe
22 months ago

Cool, thank you for being thorough :)
I may need a few days to sanitize this, since it's the holidays and I'll will need time to digest all my meals :p

Replying to andtrev:

Looks ok to me, one last thing is you need to sanitize and escape the data in the widget. Looks like you escaped output in the widget function, but not in the form function, and you should probably use WordPress sanitizing function instead of strip_tags in the update function This is a good article on doing that - http://code.tutsplus.com/articles/data-sanitization-and-validation-with-wordpress--wp-25536

After those fixes I can approve it and then they're telling me it'll take about a week for an admin to look it over and set it live, or let us know if something isn't right.

#34 @themetracbot
22 months ago

  • Summary changed from THEME: Kicoe – 1.7 to THEME: Kicoe – 1.7.1

Kicoe - 1.7.1

Kicoe is a fully responsive personal template : Colorful blog, curriculum vitae, simple content pages and personnal branding. It just keeps it simple and efficient.

Theme URL - http://wpthemes.patrickroux.fr/kicoe
Author URL - http://www.patrickoux.fr

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

Diff with previous version: https://themes.trac.wordpress.org/changeset?old_path=kicoe/1.7&new_path=kicoe/1.7.1

History:

Ticket Summary Status Resolution Owner
#22632 THEME: Kicoe – 1.4 closed not-approved poena
#26301 THEME: Kicoe – 1.8 closed live kevinhaig

(this ticket)


https://themes.svn.wordpress.org/kicoe/1.7.1/screenshot.png

#35 in reply to: ↑ 32 @kicoe
22 months ago

Hi,

Ok, I escaped the arguments on the form function.

Thank you,

Patrick

Replying to andtrev:

Looks ok to me, one last thing is you need to sanitize and escape the data in the widget. Looks like you escaped output in the widget function, but not in the form function, and you should probably use WordPress sanitizing function instead of strip_tags in the update function This is a good article on doing that - http://code.tutsplus.com/articles/data-sanitization-and-validation-with-wordpress--wp-25536

After those fixes I can approve it and then they're telling me it'll take about a week for an admin to look it over and set it live, or let us know if something isn't right.

#36 follow-up: @andtrev
22 months ago

  • Status changed from reviewing to approved

Great, I'll go ahead and set this as approved then. You may want to use esc_textarea instead of esc_html in your widget form, but since you strip tags on save maybe it's not an issue. We'll see what the admin who reviews this says.

#37 in reply to: ↑ 36 @kicoe
21 months ago

Hi,
Thank you for this. Is it possible to have an idea on when an admin will be able to take a look on my work?

Replying to andtrev:

Great, I'll go ahead and set this as approved then. You may want to use esc_textarea instead of esc_html in your widget form, but since you strip tags on save maybe it's not an issue. We'll see what the admin who reviews this says.

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


21 months ago

#39 @kevinhaig
20 months ago

I will take this final review.

#40 follow-up: @kevinhaig
20 months ago

  • Status changed from approved to reopened

Kicoe 1.7.1
Final Theme Review
======================
Here is the latest review of your theme.

The review process follows procedures found in the Theme Handbook->https://make.wordpress.org/themes/handbook/review/. If you do not understand a requirement or anything else in the review, or if you do not agree with anything, please comment in the ticket. I can then help you, or if I am not sure of something I will certainly seek a second opinion from an admin.

The second review is done to doublecheck that all requirements have been met.

Outcome

  • Theme left as reviewing for 7 days, and it may be closed after that if there is no response. Note that if you need more time, please make a comment in the ticket.
  • Sorry but I have had to reopen your theme because the following requirements have not been met.

Required

Code

  • Provide a unique prefix for everything the Theme defines in the public namespace, including options, functions, global variables, constants, post meta, etc.

Core Functionality and Features

  • The theme tags and description must match what the theme actually does in respect to functionality and design.

Language

  • All theme text strings are to be translatable.
    • Translation is missing text-domain in line 29 & 69, content-page.php.
  • I don't know why you have this code because the WordPress Language API will load local files.
    $locale = get_locale();
        $locale_file = get_template_directory() . "/languages/$locale.php";
        if (is_readable($locale_file)) {
            require_once( $locale_file );
        }
    

Licensing

  • Declare licenses of any resources included such as fonts or images, including screenshot images
    • Resources: declaration (ex Copyright 2015, by John Smith), resource download link, license type, and license download
    • Here is an example of a resource declaration specifying resource name, resource download link, copyright declaration, license type, license link. All of these things should be included, if you can get them.
      jQuery Nivo Slider v3.2, http://nivo.dev7studios.com
      Copyright 2012, Dev7studios, MIT License, http://www.opensource.org/licenses/mit-license.php
      
    • You have not declared SMINT
    • Google fonts not bundled with the theme do not need to be declared.

Naming

  • Spell “WordPress” correctly in all public facing text: all one word, with both an uppercase W and P.
    • WordPress is not spelled correctly in line 11 of /inc/footer/footer.php.

Plugins

  • Don’t do things in a theme considered plugin territory.
    • What is plugin territory is subject to different opinions at WordPress.org. If only minor content is added, or is not a functional item better suited for a plugin, it is allowed. If you don't agree with my interpretation I can get an admin to take a look, or you can ask on public slack.
    • The Kicoe Profile widget is considered plugin territory and must be removed…..sorry. Set it up in a plugin and offer it to your users either on your site or at the plugin repo on WordPress.org.

Security and Privacy

  • base64 images are not normally allowed for security reasons. Can you replace the one in jquery.lazyload.js with a bundled image?
  • Update: @otto42 just indicated that it is fine so please don't worry about it....sorry.
  • If you are a themeshop you should be selling under GPL to be in the WordPress.org repo.
    • check theme uri and author uri, all WordPress Related products need to be sold under GPL
    • I just want to make sure you understand this as I can't check your author site right now.
  • Author URI is optional. If used it is required to link to an author’s personal website or project/development website
    • Author URI returns server not found, fix it or remove it.

Stylesheets and Scripts

  • Include all scripts and resources it uses rather than hot-linking. The exception to this is Google fonts
    • remove http: from google font enqueue to maintain SSL integrity
  • If a tag is used in style.css the theme should support that feature or adhere to what that tag stands for. For example custom background or header.
    • Post Formats
      • You have added theme support for Post Formats but I am not seeing them being supported.
      • Post formats are about displaying the post in a distinctly different way from a standard post. I am not seeing that in any of your post formats.
      • if you want to continue to support post formats, make them distinctive, otherwise remove the add_theme_support() statement and ensure the tag is not in the style.css taglist

Recommended

  • Recommended items do not have to be corrected for theme approval.
    • Menu breaks out of area, see screenshot.
    • What is page-cv.php for? You have some css issues there. Did you set a page up and test it?
  • From Theme Unit Test
    • Markup: Title With Markup -> Does not work.
    • Template: Featured Image (Vertical) -> Image is too large, most of image cropped in blog index and too large in single.php
    • Template: Featured Image (Horizontal) -> Image cropped in blog index

Notes to Author

  • Theme is open for 7 days, if you need additional time to address the review make sure you make a comment to that effect in the ticket, or the theme may be closed
  • Reminders
    • Make sure you thoroughly test your changes functionally and visually.
    • Download Debug Bar by wordpressdotorg, and get it running properly, it will help you debug your theme.
    • Use the Theme Unit Test data as the basis for your testing.
    • Always test your theme in a brand new test bed to see that it loads into the admin section and displays for the first time without errors.
Last edited 20 months ago by kevinhaig (previous) (diff)

#41 @kevinhaig
20 months ago

Just to make sure I will repeat the above edit here.

@otto42 has indicated that the base64 image in in jquery.lazyload.js is fine, please ignore my earlier requirement and ....sorry.

Last edited 20 months ago by kevinhaig (previous) (diff)

#42 @kicoe
20 months ago

Thank you for this... I hope it will one day be all right, eventually :)

Just a quick question before I begin my work, about Selling, credits and links. I am not a theme shop, just a regular guy who is trying to contribute to the community. Should I then ignore this remark ?

#43 @kevinhaig
20 months ago

Sorry about all the changes :(

Yes you can ignore the "Selling under GPL" comment.

#44 in reply to: ↑ 40 @kicoe
20 months ago

Replying to kevinhaig:

Required

Code

  • Provide a unique prefix for everything the Theme defines in the public namespace, including options, functions, global variables, constants, post meta, etc.
    • theme_slug_render_title() is not prefixed

Done

  • my_wp_default_styles() is not prefixed

Done

  • load_footer() is not prefixed

Could not find any occurence of load_footer...?

Core Functionality and Features

  • The theme tags and description must match what the theme actually does in respect to functionality and design.

Done

Language

  • All theme text strings are to be translatable.
    • Translation is missing text-domain in line 29 & 69, content-page.php.

I don't know what you are talking about, and besides this file have only 41 line... Maybe you were talking about content-single ? I corrected it.

  • I don't know why you have this code because the WordPress Language API will load local files.
    $locale = get_locale();
        $locale_file = get_template_directory() . "/languages/$locale.php";
        if (is_readable($locale_file)) {
            require_once( $locale_file );
        }
    

Well, the function comments are here to explain it. Don't forget that with the unbelievably looooooooong time it took to review each of my modifications I wen through 9 versions of wordpress, maybe there are 5 or 10 lines of code in the entire theme are no more *essential* today... I essentially followed standard wordpress function : https://codex.wordpress.org/Function_Reference/load_theme_textdomain. I fail to see how this could somehow be a problem in any way : It is not a deprecated function or usage...

Licensing

  • Declare licenses of any resources included such as fonts or images, including screenshot images
    • Resources: declaration (ex Copyright 2015, by John Smith), resource download link, license type, and license download
    • Here is an example of a resource declaration specifying resource name, resource download link, copyright declaration, license type, license link. All of these things should be included, if you can get them.
      jQuery Nivo Slider v3.2, http://nivo.dev7studios.com
      Copyright 2012, Dev7studios, MIT License, http://www.opensource.org/licenses/mit-license.php
      

I already followed the template given by another reviewer. If every reviewer has a prefered template, I cannot adapt to every one of them. Requirements tells us that ressources must be declared somewhere like the readme, which is done. What is wrong with what is already done?
I declare the name of the ressource, the author, the license and the download link. I match all of the requirements, isn't it ?

  • You have not declared SMINT

Done

  • Google fonts not bundled with the theme do not need to be declared.

Doesn't need to doesn't mean cannot, right? Is it ok if I leave it ?

Naming

  • Spell “WordPress” correctly in all public facing text: all one word, with both an uppercase W and P.
    • WordPress is not spelled correctly in line 11 of /inc/footer/footer.php.

Sorry for the inconvenience...

Plugins

  • Don’t do things in a theme considered plugin territory.
    • What is plugin territory is subject to different opinions at WordPress.org. If only minor content is added, or is not a functional item better suited for a plugin, it is allowed. If you don't agree with my interpretation I can get an admin to take a look, or you can ask on public slack.
    • The Kicoe Profile widget is considered plugin territory and must be removed…..sorry. Set it up in a plugin and offer it to your users either on your site or at the plugin repo on WordPress.org.

Considered as plugin territory by who ?????

You said it yourself : "What is plugin territory is subject to different opinions". Well, if you look at previous revews from other reviewers friends of yours, it seems that your opinion about the widget is wrong and misplaced here. your own colleagues reviewer made me work days on this widget to fix it, which proves that it has its place here. It has been more than a year since severall reviewers checked it. Maintaining your comment on this directly means that reviews are random and useless. Therefore, I strongly object to this comment, based on more than a year of reviews. This widget is part of the added value of my theme, and it is fully integrated within the skin of the theme, it cannot live as a standalone plugin in the repository, it would be just garbage in the community... Please reconsider.

Stylesheets and Scripts

  • Include all scripts and resources it uses rather than hot-linking. The exception to this is Google fonts
    • remove http: from google font enqueue to maintain SSL integrity

Done

  • If a tag is used in style.css the theme should support that feature or adhere to what that tag stands for. For example custom background or header.
    • Post Formats
      • You have added theme support for Post Formats but I am not seeing them being supported.
      • Post formats are about displaying the post in a distinctly different way from a standard post. I am not seeing that in any of your post formats.
      • if you want to continue to support post formats, make them distinctive, otherwise remove the add_theme_support() statement and ensure the tag is not in the style.css taglist

Post format removed

#45 @themetracbot
20 months ago

  • Summary changed from THEME: Kicoe – 1.7.1 to THEME: Kicoe – 1.7.2

Kicoe - 1.7.2

Kicoe is a fully responsive personal template : Colorful blog, curriculum vitae, simple content pages and personnal branding. It just keeps it simple and efficient.

Theme URL - http://wpthemes.patrickroux.fr/kicoe
Author URL - http://www.patrickroux.fr

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

Diff with previous version: https://themes.trac.wordpress.org/changeset?old_path=kicoe/1.7.1&new_path=kicoe/1.7.2

History:

Ticket Summary Status Resolution Owner
#22632 THEME: Kicoe – 1.4 closed not-approved poena
#26301 THEME: Kicoe – 1.8 closed live kevinhaig

(this ticket)


https://themes.svn.wordpress.org/kicoe/1.7.2/screenshot.png

#46 @themetracbot
20 months ago

  • Summary changed from THEME: Kicoe – 1.7.2 to THEME: Kicoe – 1.7.3

Kicoe - 1.7.3

Kicoe is a fully responsive personal template : Colorful blog, curriculum vitae, simple content pages and personnal branding. It just keeps it simple and efficient.

Theme URL - http://wpthemes.patrickroux.fr/kicoe
Author URL - http://www.patrickroux.fr

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

Diff with previous version: https://themes.trac.wordpress.org/changeset?old_path=kicoe/1.7.2&new_path=kicoe/1.7.3

History:

Ticket Summary Status Resolution Owner
#22632 THEME: Kicoe – 1.4 closed not-approved poena
#26301 THEME: Kicoe – 1.8 closed live kevinhaig

(this ticket)


https://themes.svn.wordpress.org/kicoe/1.7.3/screenshot.png

#47 follow-up: @jcastaneda
20 months ago

Hi!

Taking a look over the file ( https://themes.svn.wordpress.org/kicoe/1.7.3/inc/custom-widgets.php ) I can tell there would be a lot of things that could be lost when a user changes themes which is what we try to avoid and this is why it is considered plugin territory. I do feel for you and completely understand but you also have to think about the user and their experience. Let us say they add all the fields, make the changes but when they want to change the theme they still would like to keep that information. It's not always easy migrating all those things. Yes, they exist in the database but the next theme doesn't know that information exists. Specially user profile information.

What worries me is seeing:

        $instance['kicoe-profilethumb_uri'] = $instance['kicoe-profilethumb_uri'];
        $instance['kicoe-profilethumb_id'] = $instance['kicoe-profilethumb_id'];
        $instance['kicoe-profilediapo_uri'] = $instance['kicoe-profilediapo_uri'];
        $instance['kicoe-profilediapo_id'] = $instance['kicoe-profilediapo_id'];
        //Colors
        $instance['profilebgcolor'] = esc_attr($instance['profilebgcolor']);
        $instance['profilefontcolor'] = esc_attr($instance['profilefontcolor']);
        $instance['profilecolor'] = esc_attr($instance['profilecolor']);
        $instance['profilefooterfontcolor'] = esc_attr($instance['profilefooterfontcolor']);
        //Description
        $instance['profiledescription'] = esc_html($instance['profiledescription']);
        //Social
        $instance['profilesocialintro'] = esc_html($instance['profilesocialintro']);
        $instance['profilegoogleplus'] = esc_url(['profilegoogleplus']);
        $instance['profiletwitter'] = esc_url($instance['profiletwitter']);
        $instance['profilelinkedin'] = esc_url($instance['profilelinkedin']);

In particular the $instance['profiledescription'] = esc_html($instance['profiledescription']); It makes me think of a custom post type content. There are quite a few plugins that already provide this as well:

https://wordpress.org/plugins/tags/team-members
https://wordpress.org/plugins/extra-user-details/

The key thing is really making sure that you are getting readily available content rather than have the user create it. A good way of getting that information is by using get_users
https://codex.wordpress.org/Function_Reference/get_users

Using the include parameter you can narrow things down too. A quick output could be:

<?php
// Sample array that can be populated within form()
$setting = array( 2, 14, 22 );

// Our user query
$users = get_users( array( 'include' => $setting ) );

// Our user output
foreach( $users as $user ){
        echo( get_user_meta( $user->id, 'google_url', true ) );
        echo( get_user_meta( $user->id, 'description', true ) );
}

The best part is you're not creating anything. You are using existing information/content the user already has and displaying it using not only core functionality but you aren't locking them to the theme.

Now, that is a super simplistic example because I didn't add any HTML and aren't using a widget class but it does get that information, which is what the theme should be doing.

#48 in reply to: ↑ 47 @kicoe
20 months ago

Replying to jcastaneda:

Hi!

Taking a look over the file ( https://themes.svn.wordpress.org/kicoe/1.7.3/inc/custom-widgets.php ) I can tell there would be a lot of things that could be lost when a user changes themes

Well, obviously, that is the point... If you fill something which is specifially integrated in a theme, yeah, it won't fit elsewhere. Just like nearly 90% of the themes I find on the repository, there is absolutely zero difference, I don't understand why this comes up now after 13 months of reviews...

which is what we try to avoid and this is why it is considered plugin territory.

As it is said before and as your colleague admitted, this is subjective, not objective.
I don't see why it is plugin territory, probably because it is not (at least in the opinion of the reviewers of hundreds of themes in WordPress repo that have custom widgets).

I do feel for you and completely understand

I don't think you do. I am voluntering, and as a volunteer I spent countless hours of work to match requirements for this part of my theme. I did that because my reviewers thought it was theme territory, not plugin. Period.

but you also have to think about the user and their experience.

I do. If I upload this as a standalone plugin, I will have to support styling of this widget, which has no sense here. It is fully integrated in my theme.

In particular the $instance['profiledescription'] = esc_html($instance['profiledescription']); It makes me think of a custom post type content.

Rest assured, it is not.

The key thing is really making sure that you are getting readily available content rather than have the user create it. A good way of getting that information is by using get_users

That could be useful if I was using juste these two fields you are talking about. It is like saying that if you want a car I can give you a bike wheel. No, I use more that what is available here for customization purpose. And if I have to add fields to user profile, they will not be used in other themes so the argument you used before still applies.

To conclude, I would like to repeat that I am just doing what a lot, lot themes of the repo are doing :

  • Destro has custom ads widgets, lost with theme change
  • Athena has a custom last article widget, lost with theme change
  • Supermag has advertisement widgets, lost with theme change

I could go on for hours with examples proving that you are without any doubt wrong on this matter. At least, that many other reviewer do not share your opinion about that, which is again just an opinion, not a rule to follow.
So again, this particular point can not be taken into account here. Please, reconsider.

Last edited 20 months ago by kicoe (previous) (diff)

#49 follow-ups: @kevinhaig
20 months ago

Kicoe 1.7.3
Final Theme Review
======================
Here is the latest review of your theme.

I am trying very hard to ignore your harsh critizisms and sarcastic remarks:( The Theme Review Team at WordPress.org consists of volunteers. Initial reviews are checked by senior reviewers or admins to ensure that requirements are met. Initially it is difficult for new reviewers to catch or interpret requirements. Comments like yours discourage these reviewers from continuing to donate their time.

There are a number of things you can do to help improve on the length queues. If you understand the requirements better and submit themes that adhere to these requirements the reviews go quicker. Looking through all the extensive review comments, it is apparant that was not the case. I will point out a few things below as examples of things you could have done to speed up reviews.

Once you are assigned a reviewer from the queue you can respond in a timely manner. Nine months ago, ( as you have mentioned many times ), you had an excellent reviewer Poena. If you had responded in a timely manner to her your theme would have ultimately been approved and set live. So I will not accept critizisms about a process lasting 9 months.

You can review yourself. You will learn how to be a better coder, you will also learn to understand requirements better. Helping review also helps reduce queues.

Outcome

  • Theme left as reviewing for 7 days, and it may be closed after that if there is no response. Note that if you need more time, please make a comment in the ticket.

Required

Code

  • Provide a unique prefix for everything the Theme defines in the public namespace, including options, functions, global variables, constants, post meta, etc.
    • load_footer() is not prefixed *Not fixed, look in inc/footer/footer.php, sorry loadfooter() is the correct function but then you should know that I think.

Language

  • All theme text strings are to be translatable.
    • Translation is missing text-domain in line 29 & 69, content-page.php.*fixed
    • Yes sorry…it was content-single.php. This is a good example of why reviews take longer then they should. If you paid attention to theme check then I wouldn't have to take an additional time finding the error source and then inform you of the problem.
  • I don't know why you have this code because the WordPress Language API will load local files.*not fixed but OK
    $locale = get_locale();
        $locale_file = get_template_directory() . "/languages/$locale.php";
        if (is_readable($locale_file)) {
            require_once( $locale_file );
        }
    
    • It's OK to leave it if you want. I checked with an admin because I was concerned it might affect how the Translation API works. But they said it will work fine even if it's there. I certainly don't see this code very often, so I'm pretty sure it is not needed. But your choice. Again why the sarcastic remarks? Really not necessary :(

Licensing

  • Declare licenses of any resources included such as fonts or images, including screenshot images
    • Resources: declaration (ex Copyright 2015, by John Smith), resource download link, license type, and license download
    • Here is an example of a resource declaration specifying resource name, resource download link, copyright declaration, license type, license link. All of these things should be included, if you can get them.
      jQuery Nivo Slider v3.2, http://nivo.dev7studios.com
      Copyright 2012, Dev7studios, MIT License, http://www.opensource.org/licenses/mit-license.php
      
    • You have not declared SMINT*done
    • Google fonts not bundled with the theme do not need to be declared.*Yes it really is not a big deal either way. So why take offense to the suggestion? Keep it in there if you want.
    • Yes, there is a bit of a difference among reviewers on how to declare resources. Did you bother to read the reference?
      • My comments were to try and teach both you and the initial reviewer on what is really needed.
      • The resource declaration, when supplied by the resource author should be there. You have not stated the declarations anywhere. For example Copyright (c) 2007-2013 Mika Tuupola should be added to the lazy load plugin. Licenses require this declaration. In cases like SMINT the declaration is not available so OK leave it out.
      • Most licenses also require a copy of the license summary as well. But I am OK with a link. Take a look at the MIT License link and you will see that all this information is there.
      • Normally, I like to see the declaration (if available), and resource information/links. The license link is not critical, though in my opinion it is required. Just go through and add the declarations that are available and that will be good.
  • ref: https://make.wordpress.org/themes/2014/07/08/proper-copyrightlicense-attribution-for-themes/

Naming

  • Spell “WordPress” correctly in all public facing text: all one word, with both an uppercase W and P.
    • WordPress is not spelled correctly in line 11 of /inc/footer/footer.php.*Fixed
      • Please explain why you felt this comment was necessary Sorry for the inconvienience. My interpretation is that it is critical sarcasm but I hope I am wrong!
      • WordPress takes their branding very seriously and has this requirement in there for that reason. Also remember that they provide all the resources for you to host your theme. I think they deserve a little respect for what they do.
      • This is another example where a simple check not being done has caused additional time for reviewers. But then that is what we do.

Plugins

  • Don’t do things in a theme considered plugin territory.
    • What is plugin territory is subject to different opinions at WordPress.org. If only minor content is added, or is not a functional item better suited for a plugin, it is allowed. If you don't agree with my interpretation I can get an admin to take a look, or you can ask on public slack.
    • The Kicoe Profile widget is considered plugin territory and must be removed…..sorry. Set it up in a plugin and offer it to your users either on your site or at the plugin repo on WordPress.org.*Not Done
    • I checked with @jcastaneda who is one of the admins at WordPress.org. He agrees with me that it needs to be removed.

#50 in reply to: ↑ 49 @kicoe
20 months ago

Hi,
I obviously owe you an apology, I was kind of a douche on this one... So here it is : Sorry, and thank you for taking the time.
I may need a few more days to complete the work and properly remove the widget from the theme, would this be alright ?

Replying to kevinhaig:

Kicoe 1.7.3
Final Theme Review
======================
Here is the latest review of your theme.

I am trying very hard to ignore your harsh critizisms and sarcastic remarks:( The Theme Review Team at WordPress.org consists of volunteers. Initial reviews are checked by senior reviewers or admins to ensure that requirements are met. Initially it is difficult for new reviewers to catch or interpret requirements. Comments like yours discourage these reviewers from continuing to donate their time.

There are a number of things you can do to help improve on the length queues. If you understand the requirements better and submit themes that adhere to these requirements the reviews go quicker. Looking through all the extensive review comments, it is apparant that was not the case. I will point out a few things below as examples of things you could have done to speed up reviews.

Once you are assigned a reviewer from the queue you can respond in a timely manner. Nine months ago, ( as you have mentioned many times ), you had an excellent reviewer Poena. If you had responded in a timely manner to her your theme would have ultimately been approved and set live. So I will not accept critizisms about a process lasting 9 months.

You can review yourself. You will learn how to be a better coder, you will also learn to understand requirements better. Helping review also helps reduce queues.

Outcome

  • Theme left as reviewing for 7 days, and it may be closed after that if there is no response. Note that if you need more time, please make a comment in the ticket.

Required

Code

  • Provide a unique prefix for everything the Theme defines in the public namespace, including options, functions, global variables, constants, post meta, etc.
    • load_footer() is not prefixed *Not fixed, look in inc/footer/footer.php, sorry loadfooter() is the correct function but then you should know that I think.

Language

  • All theme text strings are to be translatable.
    • Translation is missing text-domain in line 29 & 69, content-page.php.*fixed
    • Yes sorry…it was content-single.php. This is a good example of why reviews take longer then they should. If you paid attention to theme check then I wouldn't have to take an additional time finding the error source and then inform you of the problem.
  • I don't know why you have this code because the WordPress Language API will load local files.*not fixed but OK
    $locale = get_locale();
        $locale_file = get_template_directory() . "/languages/$locale.php";
        if (is_readable($locale_file)) {
            require_once( $locale_file );
        }
    
    • It's OK to leave it if you want. I checked with an admin because I was concerned it might affect how the Translation API works. But they said it will work fine even if it's there. I certainly don't see this code very often, so I'm pretty sure it is not needed. But your choice. Again why the sarcastic remarks? Really not necessary :(

Licensing

  • Declare licenses of any resources included such as fonts or images, including screenshot images
    • Resources: declaration (ex Copyright 2015, by John Smith), resource download link, license type, and license download
    • Here is an example of a resource declaration specifying resource name, resource download link, copyright declaration, license type, license link. All of these things should be included, if you can get them.
      jQuery Nivo Slider v3.2, http://nivo.dev7studios.com
      Copyright 2012, Dev7studios, MIT License, http://www.opensource.org/licenses/mit-license.php
      
    • You have not declared SMINT*done
    • Google fonts not bundled with the theme do not need to be declared.*Yes it really is not a big deal either way. So why take offense to the suggestion? Keep it in there if you want.
    • Yes, there is a bit of a difference among reviewers on how to declare resources. Did you bother to read the reference?
      • My comments were to try and teach both you and the initial reviewer on what is really needed.
      • The resource declaration, when supplied by the resource author should be there. You have not stated the declarations anywhere. For example Copyright (c) 2007-2013 Mika Tuupola should be added to the lazy load plugin. Licenses require this declaration. In cases like SMINT the declaration is not available so OK leave it out.
      • Most licenses also require a copy of the license summary as well. But I am OK with a link. Take a look at the MIT License link and you will see that all this information is there.
      • Normally, I like to see the declaration (if available), and resource information/links. The license link is not critical, though in my opinion it is required. Just go through and add the declarations that are available and that will be good.
  • ref: https://make.wordpress.org/themes/2014/07/08/proper-copyrightlicense-attribution-for-themes/

Naming

  • Spell “WordPress” correctly in all public facing text: all one word, with both an uppercase W and P.
    • WordPress is not spelled correctly in line 11 of /inc/footer/footer.php.*Fixed
      • Please explain why you felt this comment was necessary Sorry for the inconvienience. My interpretation is that it is critical sarcasm but I hope I am wrong!
      • WordPress takes their branding very seriously and has this requirement in there for that reason. Also remember that they provide all the resources for you to host your theme. I think they deserve a little respect for what they do.
      • This is another example where a simple check not being done has caused additional time for reviewers. But then that is what we do.

Plugins

  • Don’t do things in a theme considered plugin territory.
    • What is plugin territory is subject to different opinions at WordPress.org. If only minor content is added, or is not a functional item better suited for a plugin, it is allowed. If you don't agree with my interpretation I can get an admin to take a look, or you can ask on public slack.
    • The Kicoe Profile widget is considered plugin territory and must be removed…..sorry. Set it up in a plugin and offer it to your users either on your site or at the plugin repo on WordPress.org.*Not Done
    • I checked with @jcastaneda who is one of the admins at WordPress.org. He agrees with me that it needs to be removed.

#51 @kevinhaig
20 months ago

Sure .....no problem.

#52 @themetracbot
20 months ago

  • Summary changed from THEME: Kicoe – 1.7.3 to THEME: Kicoe – 1.8

Kicoe - 1.8

Kicoe is a fully responsive personal template : Colorful blog, curriculum vitae, simple content pages and personnal branding. It just keeps it simple and efficient.

Theme URL - http://wpthemes.patrickroux.fr/kicoe
Author URL - http://www.patrickroux.fr

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

Diff with previous version: https://themes.trac.wordpress.org/changeset?old_path=kicoe/1.7.3&new_path=kicoe/1.8

History:

Ticket Summary Status Resolution Owner
#22632 THEME: Kicoe – 1.4 closed not-approved poena
#26301 THEME: Kicoe – 1.8 closed live kevinhaig

(this ticket)


https://themes.svn.wordpress.org/kicoe/1.8/screenshot.png

#53 in reply to: ↑ 49 @kicoe
20 months ago

Replying to kevinhaig:

Code

  • Provide a unique prefix for everything the Theme defines in the public namespace, including options, functions, global variables, constants, post meta, etc.
    • load_footer() is not prefixed *Not fixed, look in inc/footer/footer.php, sorry loadfooter() is the correct function but then you should know that I think.

I honestly didn't see that... My bad, fixed.

Licensing

Normally, I like to see the declaration (if available), and resource information/links. The license link is not critical, though in my opinion it is required. Just go through and add the declarations that are available and that will be good.

I am really not sure I understood what the problem was, so I edited the readme file to add Copyright (c) 2007-2013 Mika Tuupola in it... Reading the link you gave me it seems that the rest is ok, I guess. Do you confirm?

Naming

  • Spell “WordPress” correctly in all public facing text: all one word, with both an uppercase W and P.
    • WordPress is not spelled correctly in line 11 of /inc/footer/footer.php.*Fixed
      • Please explain why you felt this comment was necessary Sorry for the inconvienience. My interpretation is that it is critical sarcasm but I hope I am wrong!
      • WordPress takes their branding very seriously and has this requirement in there for that reason. Also remember that they provide all the resources for you to host your theme. I think they deserve a little respect for what they do.
      • This is another example where a simple check not being done has caused additional time for reviewers. But then that is what we do.

I apologize once again for this behaviour.

Plugins

  • Don’t do things in a theme considered plugin territory.
    • What is plugin territory is subject to different opinions at WordPress.org. If only minor content is added, or is not a functional item better suited for a plugin, it is allowed. If you don't agree with my interpretation I can get an admin to take a look, or you can ask on public slack.
    • The Kicoe Profile widget is considered plugin territory and must be removed…..sorry. Set it up in a plugin and offer it to your users either on your site or at the plugin repo on WordPress.org.*Not Done
    • I checked with @jcastaneda who is one of the admins at WordPress.org. He agrees with me that it needs to be removed.

I have removed the widget, and I'll provide it as a plugin. I have seen many themes with a call to action like "This theme works better with toto or tata plugin"... Is this compliant with standards ?

#54 @kicoe
20 months ago

By the way, I forgot to edit the screenshot to remove the widget. I was about to change it now, but then I figured that it can be achieved with a text/html widget. Should I change it anyway?

#55 @kevinhaig
20 months ago

screenshot should be fine

#56 @kevinhaig
20 months ago

I am on vacation until March 5th. I will review it if I find the time otherwise it will have to wait until I get back.

#57 @kevinhaig
20 months ago

  • Owner changed from andtrev to kevinhaig
  • Status changed from reopened to reviewing

Kicoe 1.8
Final Theme Review
======================

Outcome

  • Everything looks good so I will set your theme live. Good luck with the theme :)

#58 @kevinhaig
20 months ago

  • Resolution set to live
  • Status changed from reviewing to closed
Note: See TracTickets for help on using tickets.