WordPress.org

Make WordPress Themes

Opened 2 years ago

Closed 2 years ago

#25060 closed theme (not-approved)

THEME: blankspace – 1.0

Reported by: skylerclayne Owned by: laurelfulford
Priority: new theme Keywords: theme-blankspace
Cc: skylerclayne@…

Description

blankspace - 1.0

My first public theme developed from the ground up by yours truly. Resposive design to allow for your content to be displayed on screens of all size.

Theme URL - https://wordpress.org/themes/blankspace/
Author URL - http://meetsky.me

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

History:

Ticket Summary Status Resolution Owner
#25060 THEME: blankspace – 1.0 closed not-approved laurelfulford

(this ticket)


https://themes.svn.wordpress.org/blankspace/1.0/screenshot.png

Change History (5)

#1 @laurelfulford
2 years ago

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

#2 @laurelfulford
2 years ago

Hello!

I just wanted to let you know that I've started working through your theme review, and should have something posted in the next few days!

Best,

Laurel

#3 @laurelfulford
2 years ago

Hello!

I've reviewed your theme, and have put together some feedback. The 'Required' items will need to be done before the theme can be approved.

The 'Recommended' items are only recommendations, and do not need to be completed to have your theme approved.

I know the list seems a bit long, but there's a lot of repeated items! Once the required items are completed, you can re-upload your theme here and we can continue the review.

If you have any questions at about this list, please let me know!


Required

Code

  • Sanitize everything. I noticed sanitization missing in a few spots; there may be more in the theme, please check:
    • 404.php: this code is currently commented out, but normally get_custom_header()->width and get_custom_header()->height should be escaped with esc_attr()
    • footer.php: escape echoed instances of $social_media_path with esc_url() (e.g. line 39, line 43, etc)
    • functions.php: escape get_permalink() with esc_url()
    • header.php, line 37 - $blankspace_logo should be escaped with esc_url(). Also, this code is currently commented out, but normally get_custom_header()->width and get_custom_header()->height should be escaped with esc_attr()
    • index.php, this code is currently commented out, but normally get_custom_header()->width and get_custom_header()->height should be escaped with esc_attr()


There’s some information about data validation in the Codex: https://codex.wordpress.org/Data_Validation

Frank Klein is also writing great series about escaping; the first post is linked below, and so far there are four parts:
https://make.wordpress.org/themes/2015/05/19/a-guide-to-writing-secure-themes-part-1-introduction/

  • CSS support needs to be added for the following WordPress-generated classes: alignleft, alignright, wp-caption, wp-caption-text, gallery-caption and screen-reader-text. _s can be used for examples of the styles: https://github.com/Automattic/_s

Core Functionality and Features

  • The theme tags and description must match the what the theme actually does in respect to functionality and design.
    • The theme description mentions it’s responsive, but it doesn’t behave responsively when tested on an iPhone. It looks like the theme needs viewport information added to the header.php - see _s or Twenty Fifteen for an example.
      You can also just remove 'responsive' from the description if you don't want to fix this.

Language

  • Use a single unique theme slug – as the theme slug appears in style.css.
    • In footer.php, text domain is incorrectly listed as 'blankspace-lite'
    • In comments.php, text domain is incorrectly listed as ‘twentyfifteen'
  • All theme text strings are to be translatable. Some strings that are not marked for translation are listed below - I might have missed some, so please double-check there are no others.
    • 404.php, lines 17 and 20 should be marked for translation
    • footer.php, line 70 'theme by' should be marked for translation
    • functions.php, line 29 - strings should be marked for translation
    • header.php, line 26 - strings should be marked for translation
    • home.php, line 26 - strings should be marked for translation
    • index.php, line 27 - strings should be marked for translation
    • single.php, line 26 - strings should be marked for translation

There is some examples of marking strings for translation here:
https://codex.wordpress.org/I18n_for_WordPress_Developers#Strings_for_Translation

Once all strings are marked for translation, you can generate a .pot file for the theme. There are a couple options to generated the .pot file on the WP developer website: https://developer.wordpress.org/themes/functionality/localization/#wordpress-i18n-tools.

There are also free .pot editors, like http://www.eazypo.ca/

Licensing

  • Declare licenses of any resources included such as fonts or images.
    • Please include the license information for the photo of the Toronto skyline in the README file, with the Bootstrap and Nav Walker licenses. Is this image’s license GPL compatible? If no, https://unsplash.com/ is a good resource for finding a GPL-compatible image that can be included.
    • Please include the license information for the Google fonts in the README file. The licenses can be found on the specimen pages for each font, e.g. https://www.google.com/fonts/specimen/Roboto+Slab.

Options and Settings

  • Prefix all options, custom functions, custom global variables and custom constants with the theme-slug.
    • In functions.php themename_setup should be blankspace_setup, themename_register_custom_background, should be blankspace_register_custom_background, etc. All of these functions should have the theme slug as the prefix.

Stylesheets and Scripts

  • Required to use core-bundled scripts rather than including their own version of that script. For example jQuery.
    • header.php - jQuery is included in theme and linked in header.php. This should be removed.

Recommendations

General

  • Recommend using the Site Name and Site Description from the Settings as the text that appears over the custom header image, rather than having it as part of the photo. That way, users can edit their own text WordPress, rather than editing the image.

404.php

  • Would recommend removing commented-out code on line 12, since it's not used in the theme. If it will be used, there are some notes under 'Code' about a couple things that need to be sanitized.

comments.php

  • Includes head information from Twenty Fifteen - recommend updating to ‘Blankspace'

footer.php

  • There is no closing quotes after the class social-media-icon on each social media link. This causes the links to break.
  • line 29, in this string:
    echo '<a target="_blank" class="social-media-icon href="'.esc_url(__($blankspace_socials_facebook,'blankspace-lite')).'"><img src="'. $social_media_path .'/facebook.png" \></a>
    Including __( ) around a string indicates that it should be translated. This is dynamically generated by info the user adds, so it can’t be translated. Some URLs may need translation (eg. if you want to point users to a specific language version of a site), but in this case it's not needed.
  • Social icons appear at bottom whether or not user has added them. Would recommend only displaying icons if a user has added a link to them.
  • There’s a Github icon that appears in the footer, but no option to add a link for it.
  • Would recommend handling the social media links as a ’Social Links menu', rather than in the Customizer. Twenty Fifteen has a good example of this.
  • The footer credit reads ‘theme by BLANKSPACE’ - is this correct, or do you want it to read your name? Just checking!

functions.php

  • $content_width should match the width of the content area on the website.
  • Would recommend removing commented out widget sections, starting at line 138.
  • Recommend moving required files (on lines 72, 74) - they don't need to be inside of the theme setup function.
  • Line 63 - recommend removing commented out supported post types.
  • Line 165 - adds file ‘custom-editor-style.css’, but not such file exists in the theme.
  • Line 193 - adds file ‘css/slider.css’, but the file doesn’t exist in the theme.

home.php

  • Recommend removing commented out code, as it's not used in this theme.

index.php

  • Recommend removing commented out code as it's not used in this theme.
  • Post titles and dates don’t include links. This means posts displayed on search results and archive pages that don’t have the ‘Read More’ link can’t be clicked on. Having a link on one of these other elements could be beneficial for users wanting to get a link for a specific post.

js/npm.js

  • It doesn't seem like this file is used at all, though I might be wrong! If it's not needed for the theme, would recommend removing.

style.css

Readme

  • Recommend following README format here: https://make.wordpress.org/themes/2015/04/29/a-revised-readme/ We also suggest using the .txt format instead of the .md format.
  • The Readme mentions the theme support Fontawesome. Is it possible to add some more information how it’s supported (like users can use the CSS classes from the Fontawesome documentation?)

Customizer

  • Recommend changing ‘#’ defaults in Facebook, Twitter, etc defaults to ‘https://', or leaving blank. Users who aren’t familiar with HTML might not understand that ‘#’ is used as a link placeholder.
  • The theme includes support for Header Text Colour, but there is no Header Text to change the colour of. If the Site Name and Site Description is not displayed, would recommend removing this to reduce confusion.

Next Steps

Once you've completed all of the 'Required' items we can continue with your review. Once the changes are done, just re-upload the theme on the page linked below, it will append to this ticket:

https://wordpress.org/themes/upload/

If you have any questions at all about the above, just let me know!

Best,

Laurel

#4 @laurelfulford
2 years ago

Hello!

I just wanted to follow up on this review, since no comments have been posted in seven days. It's not required that you finish all the changes in that time period, but a comment should be added to the tickets at least once a week so it's known that they're active, even if it's just a quick note that you're working on the theme.

Should the ticket be closed, you can always restart the review by resubmitting your theme.

Best,

Laurel

#5 @laurelfulford
2 years ago

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

Hello!

There haven't been any replies to this review in three weeks, so I'm going to close this ticket.

If you'd like to continue the review, please resubmit the theme with the above 'Required' changes made. The theme can be resubmitted on this webpage:

https://wordpress.org/themes/upload/

Thanks!

Laurel

Note: See TracTickets for help on using tickets.