Make WordPress Themes

Attachments (1)

logo-test.gif (787.6 KB) - added by poena 7 years ago.
the logo ( in pink) is under the post content

Download all attachments as: .zip

Change History (14)

#1 @poena
7 years ago

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

#2 @poena
7 years ago

Hi
Because the licensing issues are blockers, I have only made a partial review.

Please read all the requirements more carefully. For example, using a cdn is not allowed.
https://make.wordpress.org/themes/handbook/review/required/#stylesheets-and-scripts

Please submit your update within 7 days or request more time if needed.


License -did not pass

If you are a theme shop you should be selling under GPL to be in the WordPress.org theme directory.
It does not make a difference if the theme is free, commercial, submitted to WordPress.org or provided elsewhere
all your themes must be 100% GPL compatible.

Your site needs to state explicitly that the products you’re selling are GPL compatible.
It needs to be in an easy-to-find place for the reviewer and customers.

Themes are required to include a copyright statement for the theme itself.
Read more: https://make.wordpress.org/themes/2014/07/08/proper-copyrightlicense-attribution-for-themes/

Themes are required to be 100% GPL compatible.
Reviewers can not confirm that a theme is GPL compatible unless all the license information is included in the theme.


Readme.txt file is missing, it is required.

custom-editor-style.css is empty?

The language file refers to underscores and needs to be updated for the theme or removed.

There must not be a default logo (Any default images should not contain branding).
And the way the header image is set up as a logo it means that when I do add a logo, it is not working. It is still only showing the header image / fake logo.

Include all scripts and resources it uses rather than hotlinking.
Using a cdn or similar is not allowed. The only exception to this is Google Fonts.

All untrusted data should be escaped before output.
All instances of home_url() and get_the_post_thumbnail_url needs to be escaped with esc_url().

Translations inside HTML attributes must be escaped.
Example: function evezza_lite_search_form

Read more:
https://developer.wordpress.org/themes/theme-security/

https://developer.wordpress.org/reference/functions/esc_attr/
https://developer.wordpress.org/reference/functions/esc_attr_e/
https://developer.wordpress.org/reference/functions/esc_attr__/

https://developer.wordpress.org/reference/functions/esc_html/
https://developer.wordpress.org/reference/functions/esc_html__/
https://developer.wordpress.org/reference/functions/esc_html_e/

https://developer.wordpress.org/reference/functions/esc_url/


All theme text strings are to be translatable.
There is text in the theme that is missing translation functions.

Unfortunately there is no tool or plugin that can help you find these strings.
You will need to look through all of your files manually.
Example:
header.php line 39 "Toggle navigation".


the_title() is echoed by default and does not need to be echoed again.
See https://developer.wordpress.org/reference/functions/the_title/

#4 @themexa
7 years ago

Hi @poena , I think all the issues you mentioned are now fixed. Let me know if anything else needs to be fixed.

Regarding GPL Compatibility, we have stated in our Terms and Conditions page clearly that all our WordPress themes are 100% GPL Compatible: https://www.themexa.com/terms-and-conditions/

Regards.

#5 @poena
7 years ago

Ok, looks good, but it is hard to find the terms page when all the links in the footer are #.

#6 @themexa
7 years ago

@poena Ahh, :facepalm: . Fixed all those links. Great catch. Thanks.

@poena
7 years ago

the logo ( in pink) is under the post content

#7 @poena
7 years ago

Hi
I have now tested version 1.1


Notes

I believe all the license information is now included in the theme zip file,
but we expect a list of the assets.
It makes it a lot easier for both users and reviewers,
if we don't have to look for the licenses in several different files.
Sorry for not being clear about this. It does not have to be in the readme.txt file, but somewhere in the theme.

You can enqueue the material design icons in the same way that you enqueue google fonts,
using https://fonts.googleapis.com/icon?family=Material+Icons

In footer.php, the credits links to WordPress.org, twice.


Required:

Theme URI is invalid. It's a 404.

The logo option works now, but the logo is displayed under my content,
the size of the header area does not change with the logo size.
Is that intentional? It is problematic, because the logo still has a clickable link.
It is asking me to crop the logo at

'height'      => 160,
'width'       => 70,

So I do not know why this size is not respected. -I have not seen this happen before.

No minification of scripts or files unless you provide original files.
Themes can use minified files, but they also need to include the original, non minified version.

Make sure that the escaping functions are used correctly.
Only use esc_attr inside HTML attributes. If you want to escape content between html tags, use esc_html.

For example, the search form in functions.php line 181:
<span class="screen-reader-text">' . esc_attr( __('Search for:', 'evezza-lite') ) . '</span>
Would be
<span class="screen-reader-text">' . esc_html__('Search for:', 'evezza-lite') ) . '</span>

In archive.php
<h2><?php echo esc_attr( single_term_title() ); ?></h2>
Would be
<h2><?php echo esc_html( single_term_title() ); ?></h2>
-Instead of the query you could just use
https://developer.wordpress.org/reference/functions/the_archive_title/

In header.php
Line 35, This text is not inside an attribute:
<h3 style="color: #fff;"><?php echo esc_attr( get_bloginfo( 'name', 'display' ) ); ?></h3>

But on line 39, this text is inside an attribute:
aria-label="<?php esc_html_e( 'Toggle Navigation', 'evezza-lite' ); ?>"

Note that we are recommended, not required to escape text between HTML tags.
The reason why we are required to escape text inside HTML attributes is that unknown symbols can break the HTML.

Read more:
https://developer.wordpress.org/themes/theme-security/

https://developer.wordpress.org/reference/functions/esc_attr/
https://developer.wordpress.org/reference/functions/esc_attr_e/
https://developer.wordpress.org/reference/functions/esc_attr__/

https://developer.wordpress.org/reference/functions/esc_html/
https://developer.wordpress.org/reference/functions/esc_html__/
https://developer.wordpress.org/reference/functions/esc_html_e/

https://developer.wordpress.org/reference/functions/esc_url/

#9 @themexa
7 years ago

Updated in Version 1.1.1

the logo ( in pink) is under the post content

Fixed

I believe all the license information is now included in the theme zip file,
but we expect a list of the assets.

Fixed. All licenses are added in the assets-licenses.txt at the root directory of the theme.

You can enqueue the material design icons in the same way that you enqueue google fonts,
using https://fonts.googleapis.com/icon?family=Material+Icons

We are using the Material Design Icons, which is different. So, we have to disregard this here.

Theme URI is invalid. It's a 404.

Fixed.

The logo option works now, but the logo is displayed under my content

Fixed.

No minification of scripts or files unless you provide original files.

Fixed.

Make sure that the escaping functions are used correctly.

Fixed.

Instead of the query you could just use https://developer.wordpress.org/reference/functions/the_archive_title/

Unfortunately, we cannot use the_archive_title() because it prints the term type before the term title, e.g. "Category: Uncategoried", whereas we only want "Uncategorized". Here, single_term_title() serves our purpose.

In header.php
Line 35, This text is not inside an attribute:
But on line 39, this text is inside an attribute:

Fixed.

Please let me know if anything else is required.

Regards.

#10 @poena
7 years ago

Hi
I am sorry for the wait.
I have checked version 1.1.1 and I believe everything I mentioned has been fixed.

There are some minor things left:
evezza-lite/inc/template-tags.php
29 ERROR Strings should have translatable content
45 ERROR Strings should have translatable content
-There is nothing there to translate.

In header.php, line 39, aria-label="<?php esc_attr( 'Toggle Navigation', 'evezza-lite' ); ?>"
esc_attr alone does not echo or translate anything.
You can use esc_attr_e() to both escape, translate and echo the text.

Remove the development files (phpcs, webpack, package.json etc). These are more suitable to keep at for example github.
For example, the custom phpcs file is causing havoc with my code editors PHPCS settings :)

#11 @themexa
7 years ago

Hi @poena

evezza-lite/inc/template-tags.php
29 ERROR Strings should have translatable content
45 ERROR Strings should have translatable content
-There is nothing there to translate.

Fixed

In header.php, line 39, aria-label="<?php esc_attr( 'Toggle Navigation', 'evezza-lite' ); ?>"
esc_attr alone does not echo or translate anything.

Fixed

Remove the development files (phpcs, webpack, package.json etc).

Fixed

Please let me know if anything else is required.

Regards.

Last edited 7 years ago by themexa (previous) (diff)

#13 @poena
7 years ago

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

Hi
Thank you for the update. I will set the theme live, and it should show up in the directory shortly.

Note: See TracTickets for help on using tickets.