WordPress.org

Make WordPress Themes

Opened 3 years ago

Closed 3 years ago

#21352 closed theme (not-approved)

THEME: I Am One - 1.3.7

Reported by: sonalsinha21 Owned by: Frank Klein
Priority: theme update Keywords: theme-i-am-one
Cc: support@…

Description

I Am One - 1.3.5

I Am One is an elegant and easiest to use Responsive WordPress theme with all kinds of elements available in order to complete a single one page website. It is best suited for any kind of industry like designers, design firms, business houses, corporates, personal profiles etc. Comes with 5 default templates, default parallax slider, social media integration, default gallery and widgets for blog area and other pages. It is translation ready. The theme is compatible with popular plugins like WooCommerce, bbPress and Contact form 7. Demo: http://sktthemesdemo.net/iamone

Theme URL - http://www.sktthemes.net/themes/iamone/
Author URL - http://www.sktthemes.net

SVN - https://themes.svn.wordpress.org/i-am-one/1.3.5
ZIP - https://wordpress.org/themes/download/i-am-one.1.3.5.zip?nostats=1

Diff with previous version: https://themes.trac.wordpress.org/changeset?old_path=/i-am-one/1.3.4&new_path=/i-am-one/1.3.5

History:

Ticket Summary Status Resolution Owner
#20014 THEME: I Am One - 1.2.1 closed closed-newer-version-uploaded sittestaccount
#20548 THEME: I Am One - 1.3.4 closed live sittestaccount
#21352 THEME: I Am One - 1.3.7 closed not-approved Frank Klein

(this ticket)

#21644 THEME: I Am One - 1.3.8 closed live downstairsdev
#21960 THEME: I Am One - 1.3.9 closed live karmatosed
#22391 THEME: I Am One - 1.4 closed live jcastaneda
#22502 THEME: I Am One - 1.4.1 closed live karmatosed
#23086 THEME: I Am One - 1.5 closed live jcastaneda
#24989 THEME: I Am One – 1.5.1 closed live karmatosed
#28425 THEME: I Am One – 1.5.2 closed live jcastaneda


https://themes.svn.wordpress.org/i-am-one/1.3.5/screenshot.png

Change History (18)

#1 @Frank Klein
3 years ago

  • Owner set to Frank Klein
  • Status changed from new to reviewing

#2 follow-ups: @Frank Klein
3 years ago

Required

index.php

This template is a fallback template and should contain a standard loop, like you have implemented at the top of the file.

The rest of the code though should be moved to a custom page template.

Getting options

When using of_get_option(), the default value should align with the option you are getting.

So for of_get_option('sectiontitle'.$s, true) != '' ), the default should not be true, but '', since that is what you are comparing against. For of_get_option('numsection', true) > 0, it should be an absolute integer, since you are comparing whether it is larger than 0.

Please review the theme and correct these option calls.

Escaping options

  • numsection and other integers should be escaped using absint().
  • Options output in HTML attributes should be escaped using esc_attr().
  • Options output into the template HTML should be escaped using esc_html().
  • Options that return URLs should be escaped using esc_url().

Please review the theme and adapt or add missing escaping.

functions.php

  • skt_iamone_string_limit_words() and skt_iamone_get_slug_by_id seem to be unused by the theme, please remove these functions if it are not used by the theme.
  • skt_iamone_scripts(): for themes that are marked translation-ready, fonts need to be enqueued in a way that allows them to be deactivated by translators if the font does not support the character set of their language. See: http://themeshaper.com/2014/08/13/how-to-add-google-fonts-to-wordpress-themes/
  • skt_iamone_media_css_hook(): The stylesheets need to be enqueued.
  • skt_iamone_media_css_hook(): The Javascript should be placed in a dedicated file and enqueued. You can use wp_localize_script() to pass values from PHP to this Javascript file.
  • skt_iamone_credit_link(): Please make the text available for translation.

custom-functions.php

  • skt_iamone_optionsframework_custom_scripts() needs to be placed in a Javascript file and properly enqueued in the admin.
  • skt_iamone_hook_custom_javascript() needs to be placed in a Javascript file and enqueued.
  • skt_iamone_get_post_categories(): Please make all text available for translation.
  • SKT_THEME_URL and SKT_URL seem to be unused by the theme, please remove these constants if the theme doesn't use them.

custom.js

All variables living in the global namespace need to be prefixed or removed from the global namespace by placing them in an self-executing anonymous function.

Miscellaneous

  • content-home.php seems to be unused by the theme. Please remove this file if it isn't used by the theme.

Escaping

You should escape as late as possible, ideally before output. So instead of doing this:

$class = esc_html( of_get_option( 'sectionclass'.$s, true) );
(5 lines of code)
<div class="<?php echo $class; ?>'>

It would be easier to read like this:

$class = esc_html( of_get_option( 'sectionclass'.$s, true) );
(5 lines of code)
<div class="<?php echo esc_html( $class ); ?>">

Getting options

$title = ( of_get_option('sectiontitle'.$s, true) != '' ) ? esc_html( of_get_option('sectiontitle'.$s, true) ) : '';

Code like the above code can be simplified:

  1. There is no need for the $title variable, as you can simple call the of_get_option() function when needed.
  2. The ternary conditional is not needed, since of_get_option() will return the default value of the option isn't set.

So the line could simply be of_get_option( 'sectiontitle'.$s, '' ) and be used instead of $title.

So for the code in the previous section, it would be:

<div class="<?php echo esc_attr( of_get_option( 'sectionclass'.$s, '' ) ); ?>">

functions.php
In skt_iamone_custom_head_codes(), esc_html() is used to escape the custom CSS. This will break any CSS using quotes or > for example.

With CSS, the critical part is really the sanitization, so as long as that is covered, the CSS should be safe to output.

Also this option should only be available to users having the unfiltered_html capability.

#3 @sonalsinha21
3 years ago

Hi,

Thanks for the review.

Give us 2 days since there is a lot of comments from your end.

#4 @themetracbot
3 years ago

  • Summary changed from THEME: I Am One - 1.3.5 to THEME: I Am One - 1.3.6

I Am One - 1.3.6

I Am One is an elegant and easiest to use Responsive WordPress theme with all kinds of elements available in order to complete a single one page website. It is best suited for any kind of industry like designers, design firms, business houses, corporates, personal profiles etc. Comes with 5 default templates, default parallax slider, social media integration, default gallery and widgets for blog area and other pages. It is translation ready. The theme is compatible with popular plugins like WooCommerce, bbPress and Contact form 7. Demo: http://sktthemesdemo.net/iamone

Theme URL - http://www.sktthemes.net/themes/iamone/
Author URL - http://www.sktthemes.net

SVN - https://themes.svn.wordpress.org/i-am-one/1.3.6
ZIP - https://wordpress.org/themes/download/i-am-one.1.3.6.zip?nostats=1

Diff with previous version: https://themes.trac.wordpress.org/changeset?old_path=/i-am-one/1.3.5&new_path=/i-am-one/1.3.6

History:

Ticket Summary Status Resolution Owner
#20014 THEME: I Am One - 1.2.1 closed closed-newer-version-uploaded sittestaccount
#20548 THEME: I Am One - 1.3.4 closed live sittestaccount
#21352 THEME: I Am One - 1.3.7 closed not-approved Frank Klein

(this ticket)

#21644 THEME: I Am One - 1.3.8 closed live downstairsdev
#21960 THEME: I Am One - 1.3.9 closed live karmatosed
#22391 THEME: I Am One - 1.4 closed live jcastaneda
#22502 THEME: I Am One - 1.4.1 closed live karmatosed
#23086 THEME: I Am One - 1.5 closed live jcastaneda
#24989 THEME: I Am One – 1.5.1 closed live karmatosed
#28425 THEME: I Am One – 1.5.2 closed live jcastaneda


https://themes.svn.wordpress.org/i-am-one/1.3.6/screenshot.png

#5 in reply to: ↑ 2 @sonalsinha21
3 years ago

Replying to Frank Klein:
Hi Frank,

Updated the theme with all the required and recommended you mentioned.

Waiting for your next response.

Regards,
Shri

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

Replying to Frank Klein:
Hey Frank,

Hope you are doing good. Waiting for your next response or update. Hope now the theme is all fine and can be approved.

Regards,
Shri

#7 follow-ups: @Frank Klein
3 years ago

Required

The following items has not been resolved yet.

js/custom.js

  • The variables navwidth, adjustMenu, and ss need to be prefixed with the theme slug, as they are global variables.

inc/custom-functions.php

  • skt_iamone_optionsframework_custom_scripts() needs to be enqueued on admin_enqueue_scripts (with an "s" at the end!). This script should also be enqueued just on the option page. Like this:
function skt_iamone_optionsframework_custom_scripts( $hook ) {
    if ( 'appearance_page_iamone-options' != $hook ) {
	return;
    }
    wp_enqueue_script( 'option-script', get_template_directory_uri().'/js/optionframework-custom.js' );
}
add_action( 'admin_enqueue_scripts', 'skt_iamone_optionsframework_custom_scripts' );
  • skt_iamone_hook_custom_javascript() needs to be enqueued on wp_enqueue_scripts.
  • skt_iamone_get_post_categories(): The text "Categories" needs to be made available for translation.

functions.php

  • skt_iamone_credit_link(): Thanks for making the text available for translation, but you can't use functions inside translation functions. So it needs to be: printf( __('I am one theme by <a href="%s" target="_blank">SKTThemes.</a>', 'skt-iamone' ), esc_url('http://www.sktthemes.net') );
  • skt_iamone_media_css_hook(): The stylesheets need to be enqueued on wp_enqueue_scripts.
  • skt_iamone_media_css_hook(): The Javascript needs to be placed in a separate file and enqueued properly.

Getting options

When using of_get_option(), the default value should align with the option you are getting.

So for of_get_option('sectiontitle'.$s, true) != '' ), the default should not be true, but '', since that is what you are comparing against. For of_get_option('numsection', true) > 0, it should be an absolute integer, since you are comparing whether it is larger than 0.

Escaping options

  • numsection and other integers should be escaped using absint().
  • Options output in HTML attributes should be escaped using esc_attr().
  • Options output into the template HTML should be escaped using esc_html().
  • Options that return URLs should be escaped using esc_url().

#8 @themetracbot
3 years ago

  • Summary changed from THEME: I Am One - 1.3.6 to THEME: I Am One - 1.3.7

I Am One - 1.3.7

I Am One is an elegant and easiest to use Responsive WordPress theme with all kinds of elements available in order to complete a single one page website. It is best suited for any kind of industry like designers, design firms, business houses, corporates, personal profiles etc. Comes with 5 default templates, default parallax slider, social media integration, default gallery and widgets for blog area and other pages. It is translation ready. The theme is compatible with popular plugins like WooCommerce, bbPress and Contact form 7. Demo: http://sktthemesdemo.net/iamone

Theme URL - http://www.sktthemes.net/themes/iamone/
Author URL - http://www.sktthemes.net

SVN - https://themes.svn.wordpress.org/i-am-one/1.3.7
ZIP - https://wordpress.org/themes/download/i-am-one.1.3.7.zip?nostats=1

Diff with previous version: https://themes.trac.wordpress.org/changeset?old_path=/i-am-one/1.3.6&new_path=/i-am-one/1.3.7

History:

Ticket Summary Status Resolution Owner
#20014 THEME: I Am One - 1.2.1 closed closed-newer-version-uploaded sittestaccount
#20548 THEME: I Am One - 1.3.4 closed live sittestaccount
#21352 THEME: I Am One - 1.3.7 closed not-approved Frank Klein

(this ticket)

#21644 THEME: I Am One - 1.3.8 closed live downstairsdev
#21960 THEME: I Am One - 1.3.9 closed live karmatosed
#22391 THEME: I Am One - 1.4 closed live jcastaneda
#22502 THEME: I Am One - 1.4.1 closed live karmatosed
#23086 THEME: I Am One - 1.5 closed live jcastaneda
#24989 THEME: I Am One – 1.5.1 closed live karmatosed
#28425 THEME: I Am One – 1.5.2 closed live jcastaneda


https://themes.svn.wordpress.org/i-am-one/1.3.7/screenshot.png

#9 in reply to: ↑ 7 @sonalsinha21
3 years ago

Replying to Frank Klein:
All changes done.

Regards,
Shri

#10 in reply to: ↑ 7 @sonalsinha21
3 years ago

Replying to Frank Klein:
Hi Frank,

Its been 5 days waiting for your response.

Regards,
Shri

#11 follow-ups: @Frank Klein
3 years ago

Required

This is now the third time that I have to point out the same issues in this review. If you don't understand a requirement, please ask and I will explain.

But please apply yourself. I gave you a a code snippet for a correct implementation of the footer credits. You did not even correct this error, although all you had to do was copy and paste this.

I am here to help, but I'm not your quality assurance. If you do not correct all the required items in the next revision, I will drop this review and you will have to wait on another reviewer.

The following items have not been resolved yet.

js/custom.js

  • The variable adjustMenu needs to be prefixed with the theme slug, as they are global variables.

inc/custom-functions.php

  • skt_iamone_hook_custom_javascript() needs to be enqueued on wp_enqueue_scripts.

functions.php

  • skt_iamone_credit_link(): Thanks for making the text available for translation, but you can't use functions inside translation functions. So it needs to be: printf( __('I am one theme by <a href="%s" target="_blank">SKTThemes.</a>', 'skt-iamone' ), esc_url('http://www.sktthemes.net') );
  • skt_iamone_media_css_hook(): The stylesheets need to be enqueued on wp_enqueue_scripts.
  • skt_iamone_media_css_hook(): The Javascript needs to be placed in a separate file and enqueued properly.

Getting options

When using of_get_option(), the default value should align with the option you are getting.

So for of_get_option('sectiontitle'.$s, true) != '' ), the default should not be true, but '', since that is what you are comparing against. For of_get_option('numsection', true) > 0, it should be an absolute integer, since you are comparing whether it is larger than 0.

#12 in reply to: ↑ 11 @sonalsinha21
3 years ago

Replying to Frank Klein:
Hi Frank,

This was a theme update and seriously we didn't expect so many revisions and points. However we tried to do as you said everything.

Sorry about some part missing. Only a handful were missed if you check there were many.

Once you send us review we send you back the response within the same day.

If you can check it the same day it will be good.

I am resending you the theme once everything is done.

Regards,
Shri

#13 in reply to: ↑ 11 @sonalsinha21
3 years ago

Replying to Frank Klein:
inc/custom-functions.php

  • skt_iamone_hook_custom_javascript() needs to be enqueued on

`wp_enqueue_scripts
We have already placed the javascript on separate file and and enqueued within the skt_iamone_hook_custom_function so i am not sure where we did anything wrong. (Are you checking version 1.3.7?)

  • skt_iamone_media_css_hook(): The stylesheets need to be enqueued on

wp_enqueue_scripts.
We have used wp_enqueue_style if we use what you are asking: wp_enqueue_scripts then they break so not sure how i am supposed to use them.

skt_iamone_media_css_hook(): since its part of functions we aren't able to place it in separate file. Again offer us some solution or what should we do here?

So for of_get_option('sectiontitle'.$s, true) != '' ), the default
should not be true, but '', since that is what you are comparing
against. For of_get_option('numsection', true) > 0, it should be an
absolute integer, since you are comparing whether it is larger than 0.


Regards,
Shri

#14 in reply to: ↑ 11 @sonalsinha21
3 years ago

Replying to Frank Klein:
Hi Frank,

Have answered your queries above and wait for your response.

Regards,
Shri

#15 in reply to: ↑ 11 @sonalsinha21
3 years ago

Replying to Frank Klein:
Hi Frank,

Have wrote back my comments to you above.

Wait for your update on those points.

Regards,
Shri

#16 follow-up: @Frank Klein
3 years ago

Here my responses to your questions:

skt_iamone_hook_custom_javascript() needs to be enqueued on wp_enqueue_scripts.

You are using this: add_action('wp_head','skt_iamone_hook_custom_javascript');, but it needs to be add_action( 'wp_enqueue_scripts', 'skt_iamone_hook_custom_javascript' );

skt_iamone_media_css_hook(): The stylesheets need to be enqueued on wp_enqueue_scripts.

The function needs to look like this:

function skt_iamone_media_css_hook() {
	wp_enqueue_style('skt-iamone-computer-style', get_template_directory_uri().'/css/computer.css','','','screen and (min-width:940px)');
	wp_enqueue_style('skt-iamone-tab-style', get_template_directory_uri().'/css/tablet.css','','','screen and (min-width: 720px) and (max-width:939px)');
	wp_enqueue_style('skt-iamone-mobhor-style', get_template_directory_uri().'/css/mobile_hz.css','','','screen and (min-width: 480px) and (max-width:719px)');
	wp_enqueue_style('skt-iamone-mobile-style', get_template_directory_uri().'/css/mobile.css','','','screen and (max-width: 479px)');
}
add_action( 'wp_enqueue_scripts', 'skt_iamone_media_css_hook' );

skt_iamone_media_css_hook(): The Javascript needs to be placed in a separate file and enqueued properly.

This Javascript code:

jQuery(document).ready(function(){ 
        jQuery.supersized({
                // Functionality
                slideshow               :   1,			// Slideshow on/off
                autoplay				:	1,			// Determines whether slideshow begins playing when page is loaded. 
                start_slide             :   1,			// Start slide (0 is random)
                stop_loop				:	0,			// Pauses slideshow on last slide
                random					: 	0,			// Randomize slide order (Ignores start slide)
                slide_interval          :   5000,		// Length between transitions
                transition              :   1, 			// 0-None, 1-Fade, 2-Slide Top, 3-Slide Right, 4-Slide Bottom, 5-Slide Left, 6-Carousel Right, 7-Carousel Left
                transition_speed		:	1000,		// Speed of transition
                new_window				:	1,			// Image links open in new window/tab
                pause_hover             :   0,			// Pause slideshow on hover
                keyboard_nav            :   1,			// Keyboard navigation on/off
                performance				:	1,			// 0-Normal, 1-Hybrid speed/quality, 2-Optimizes image quality, 3-Optimizes transition speed // (Only works for Firefox/IE, not Webkit)
                image_protect			:	0,			// Disables image dragging and right click with Javascript
                
                // Size & Position
                min_width		        :   0,			// Min width allowed (in pixels)
                min_height		        :   0,			// Min height allowed (in pixels)
                vertical_center         :   1,			// Vertically center background
                horizontal_center       :   1,			// Horizontally center background
                fit_always				:	0,			// Image will never exceed browser width or height (Ignores min. dimensions)
                fit_portrait         	:   1,			// Portrait images will not exceed browser height
                fit_landscape			:   0,			// Landscape images will not exceed browser width
                
                // Components 				
                slide_links				:	'blank',	// Individual links for each slide (Options: false, 'num', 'name', 'blank')
                thumb_links				:	1,			// Individual thumb links for each slide
                thumbnail_navigation    :   0,			// Thumbnail navigation
                slides 					:  	[			// Slideshow Images
                                                                                <?php
                                                                                $n = 0;
                                                                                foreach( $slAr as  $sv ){
                                                                                        $n++;
                                                                                        echo "{image : '".$sv."', title : '', thumb : '".$sv."', url : ''}".( (count($slAr) == $n) ? '' : ',' )."\n";
                                                                                }
                                                                                ?>
                                                                        ],
                // Theme Options 
                progress_bar			:	1,			// Timer for each slide							
                mouse_scrub				:	0
        });
});

Needs to be placed in a Javascript file and the PHP code needs to be removed.

Then you need to enqueue this Javascript file, like you enqueued the other Javascript files, like hook-custom-script.js for example.

And finally you need to rework this Javascript, so that it works fine without the PHP you removed from it.

When these changes break something, then you have to rework your code to get it working. There is no reason why enqueuing scripts and styles would break a theme.

#17 in reply to: ↑ 16 @sonalsinha21
3 years ago

Replying to Frank Klein:
Hi,

The 2 other points we were able to do but we have to apply php within javascript of skt_iamone_media_css_hook() which you asked us to place in other file. If we keep that there then we are forced to use htaccess because parsing php in javascript requires that.

Can't think of any other way.

Kindly close this ticket.

We will update the theme later on.

#18 @Frank Klein
3 years ago

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

The 2 other points we were able to do but we have to apply php within javascript of skt_iamone_media_css_hook() which you asked us to place in other file. If we keep that there then we are forced to use htaccess because parsing php in javascript requires that.

A solution was proposed in the first review feedback provided for the theme: You can use wp_localize_script() to pass values from PHP to this Javascript file.

What this means is that it is possible to generate the necessary values in PHP, and then use the wp_localize_script() function to pass these values to be used in Javascript by the slider initialization code.

The current method of implementation is not correct, and it is a required code quality item to do this the correct way. So any solution that just outputs a blob of code to the head element will not pass the review.

Marking this as not approved based on your request.

Note: See TracTickets for help on using tickets.