Created
November 5, 2012 02:09
-
-
Save mfields/4014915 to your computer and use it in GitHub Desktop.
Theme Review of Black Zebra for Extend.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Need to be fixed for inclusion | |
------------------------------ | |
* The function ```register_sidebar()``` needs to be called during the ```widgets_init``` hook. Please refer to Twenty Twelve's function.php file for an example of how to do this. | |
* The template tag ```the_title()``` may not be printed inside an attribute. Please use ```the_title_attribute()``` instead. This code was found in line 14 of 404.php, line 11 of archive.php, line 9 of header.php, line 11 of index.php, line 11 of page.php, line 13 of search.php, and line 11 of single.php. | |
* Please move the function definition for ```blackzebra_custom_styles()``` into functions.php. | |
* Please remove the additional enqueue for a child theme from ```blackzebra_custom_styles()```. It is best to let the child theme make this decision. | |
* Please remove the favicon from header.php. | |
* Please remove the Atom Link from header.php. This functionality should be taken care of by ```add_theme_support( 'automatic-feed-links' )``` | |
* Table headings are the same color as their background: http://download.mfields.org/screenshot/2012-11-04_1609.png Please rework the styles so that they are readable. | |
* "Comments are Closed" message should not display on pages having no comments. http://download.mfields.org/screenshot/2012-11-04_1611.png | |
* There is no way to navigate between posts shown by single.php. | |
* All heading tags should clear floats. http://download.mfields.org/screenshot/2012-11-04_1614.png | |
* Elements that depend on ```$content_width``` bust out of their containers. Please set ```$content_width``` to 800 instead of 900. | |
* The screen shot shows that there are four columns in the footer section that appear to be widget areas. This is what the theme looks like when I first install it with no custom widgets enabled: http://download.mfields.org/screenshot/2012-11-04_1717.png When I install a few custom widgets, this area is converted to one column: http://download.mfields.org/screenshot/2012-11-04_1722.png I think it would be best to chose one design or the other. Having both might be confusing to users. | |
* The header section does not contain the menu when there are a lot of links. | |
* Images have a max width of 510 pixels. This means that larger images are not allowed to fill the full-width of the content area and may appear distorted: http://download.mfields.org/screenshot/2012-11-04_1759.png | |
* Floats are not contained in the posts/pages wrapper: http://download.mfields.org/screenshot/2012-11-04_1801.png I like to use code like this: https://gist.github.com/3950905 | |
* Both ordered and unordered list do not have proper indicators and display with borders when shown in comments. http://download.mfields.org/screenshot/2012-11-04_1804.png | |
* Block-level items that appear in the comment body have now margins making them squish together: http://download.mfields.org/screenshot/2012-11-04_1806.png | |
* Although there are two theme locations for custom menus registered, no call to ```wp_nav_menu()``` could be found in the theme. Please add this function where appropriate. | |
Please fix in your next version | |
------------------------------- | |
* Please hook all code in function.php into the ``after_setup_theme``` action, This includes setting of ```$content_width```, call to ```register_nav_menus()```, ```the calls to add_theme_support```, and the ```blackzebra_translate()``` function. | |
* There is no need to check for existence of the following functions: | |
* ```register_sidebar()```, ```register_nav_menus```, and ```add_theme_support()```. The conditional checks can be removed. | |
* There appears to be an unneeded call to ```get_sidebar()``` in 404.php. This can be removed because it is called in footer.php. | |
* Smilies are contained in a large black box. Since these are inline elements intended to be used in passages of text, it would be best to remove this box: http://download.mfields.org/screenshot/2012-11-04_1628.png | |
Recommended | |
----------- | |
* Please move the enqueue of the comments reply into a function in functions.php hooked into the ```comment_form_before``` action. There is a <a href="http://make.wordpress.org/themes/2012/05/08/proposed-wordpress-3-4-guidelines-revisions/">code snippet here</a>. | |
* The ```function_exists()``` check for ```dynamic_sidebar()``` in sidebar.php is not really necessary and can be safely removed. | |
Nice-to-have | |
------------ | |
These items are merely suggestions and have no bearing on the team review process. | |
* The text "blackzebra is have one column" in the theme description would better read "blackzebra has one column". | |
* For best possible user experience, it is best to not set the value of the search input to "search". The html5 placeholder attribute is a much better choice in this situation. Please see the example in underscores: https://github.com/Automattic/_s/blob/master/searchform.php |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment