WordPress.org

Theme Review Team

Recent Updates Page 3 Toggle Comment Threads | Keyboard Shortcuts

  • Jose Castaneda 10:01 pm on February 17, 2015 Permalink |  

    Handbook Examples: Functionality and Features 

    Last week we began with the code section of the handbook. This week let’s get some momentum and tackle: Core functionality and features.

    Functionality and Features examples:

    Example:
    There is no need to use
    add_theme_support( 'menus' );
    because register_nav_menu() does it behind the scenes.

    Example:
    Use core defined functions when possible
    So rather than using your own functions:

    function my_custom_cats(){
    	$categories = get_the_category();
    	$catOut = '';
    	$separator = ', ';
    	$catOutput = '';
    	if($categories){
    		foreach($categories as $category) {
    			$catOutput .= '<a href="'.get_category_link( $category->term_id ).'" title="' . esc_attr( sprintf( __( "View all posts in %s", 'text-domain' ), $category->name ) ) . '">'.$category->cat_name.'</a>'.$separator;
    		}
    		$catOut = 'Categories: '.trim($catOutput, $separator);
    	}
    	return $catOut;
    }
    

    Use a core function like the_category()

     
    • Justin Tadlock 10:35 pm on February 23, 2015 Permalink | Log in to Reply

      Use get_template_directory() rather than TEMPLATEPATH to return the template path.

      require_once( trailingslashit( get_template_directory() ) . 'inc/example.php' );

      Use get_stylesheet_directory() rather than STYLESHEETPATH to return the stylesheet path.

      require_once( trailingslashit( get_stylesheet_directory() ) . 'inc/example.php' );

      This example should come with an additional warning to check if the file exists first if not used in a child theme.

      Include comments_template().

      comments_template();

      Should be called in at least all singular views.

  • Jose Castaneda 7:30 pm on February 17, 2015 Permalink |  

    Chat Notes 

    Slack Archives: https://wordpress.slack.com/archives/themereview/p1424196039002725

    Topics:

    Queue

    • some improvement in last 5/6 months
    • button assigning

    Frameworks

    • can take long time
    • ‘sos’ checkbox ( looking at you #meta team )
    • education ( tuts and stuff )

    Child theme plugins

    • add support for plugin but not styling
    • not allowed
     
  • Jose Castaneda 8:38 pm on February 16, 2015 Permalink |  

    Chat Agenda

    Topic: Frameworks

    If you have any other topics you would like to discuss please post them here.

    Looking forward to seeing you there! Same time and place: Slack #themereview 1800UTC

     
  • Justin Tadlock 9:19 pm on February 10, 2015 Permalink |  

    Handbook Examples: Code Section 

    Beginning today, we will be discussing examples for the handbook requirements. Ultimately, these examples will end up on the Explanations and Examples page in the handbook.

    Each week, we’ll focus on various sections. This week’s section is the Code section. What we need from everyone is to provide examples in the below comments. The admins will round up the examples and add them to the handbook.

    Code section examples

    I’ll get us started with a couple of examples just to get the ball rolling.

    Required: Have a valid DOCTYPE declaration and include language_attributes.

    <!DOCTYPE html>
    <html <?php language_attributes(); ?>>

    Required: No shortcodes are allowed.

    The use of the add_shortcode() function is not allowed in themes as shown below:

    add_shortcode( 'shortcode_name', 'shortcode_callback_func' );
     
  • Justin Tadlock 8:57 pm on February 10, 2015 Permalink |  

    Custom CSS boxes in themes 

    Recently, we’ve had a big discussion over whether themes could allow users to enter CSS into arbitrary text boxes (custom CSS). There was some confusion over what was allowed and, if allowed, what would be the proper method of handling it. After much debate, we’ve come to the following conclusion:

    • It’s preferred that theme authors leave this feature to plugins.
    • However, it is allowed if handled safely.
    • The edit_theme_options capability is required (like all theme options).
    • The wp_filter_nohtml_kses(), wp_strip_all_tags(), or equivalent function must be used to sanitize the data before it’s saved into the database.

    Why leave to plugins?

    There are cases where it makes sense for custom CSS to be applied globally and to be applied on a per-theme basis. Generally-speaking, that’s a decision that’s best left up to the user, which they can decide based on the various plugins available for handling this.

    Validating CSS. Our guidelines are not going to require that theme authors validate the CSS. Our guidelines are only concerned with making sure the data is safe. That means that users could add in arbitrary code that’s not valid CSS. This means it’d be on you, the theme author, to handle the inevitable support requests.

    You are, of course, welcome to use a library such as CSSTidy to tidy the CSS up, but that’s a large library and probably best left in plugins. Just realize that the more complex your theme code is the harder it is to review and the longer it’ll take us.

    Edit: Another good reason to leave this out of themes is multisite. There are many instances where a site owner wouldn’t want everyone with a blog on their site to have this sort of CSS editing capability. Adding this option will most likely mean that a good number of multisite installs will never use your theme.

    Edit #2: To add to the multisite scenario, it might be worth considering using the edit_themes capability instead of edit_theme_options.

    How to do it correctly?

    I’m going to share an example of handling this correctly based on the comments from our recent discussion. This is only going to focus on the Customizer because it’s our recommended way to handle theme options.

    The following is example code to start from:

    <?php 
    
    add_action( 'customize_register', 'prefix_theme_customize_register' );
    
    function prefix_theme_customize_register( $wp_customize ) {
    
    	$wp_customize->add_section(
    		'prefix_section_1',
    		array(
    			'title'       => __( 'Section 1', 'prefix' ),
    			'description' => __( 'Section 1 description.', 'prefix' ),
    			'priority'    => 30
    		)
    	);
    
    	$wp_customize->add_setting(
    		'example_css',
    		array(
    			'default'              => '',
    			'capability'           => 'edit_theme_options',
    			'sanitize_callback'    => 'wp_filter_nohtml_kses',
    			'sanitize_js_callback' => 'wp_filter_nohtml_kses'
    		)
    	);
    
    	$wp_customize->add_control(
    		'prefix_example_css_control',
    		array(
    			'label'    => __( 'Custom CSS', 'prefix' ),
    			'section'  => 'prefix_section_1',
    			'settings' => 'example_css',
    			'type'     => 'textarea'
    		)
    	);
    }
     
    • Ryan Hellyer 9:03 pm on February 10, 2015 Permalink | Log in to Reply

      For anyone looking for an example of correctly handling CSS, I recommend checking out the Safe CSS plugin from Automattic …
      https://wordpress.org/plugins/safecss/

      It handles both CSS validation and sanitization. As Justin points out above, this does result in a rather large code base.

      • Ryan Hellyer 9:04 pm on February 10, 2015 Permalink | Log in to Reply

        I am referring here to how the data is handled BTW, not the UI which is best handled the way Justin described above.

        • Zane Matthew 12:47 am on February 28, 2015 Permalink | Log in to Reply

          So your saying the best way to store CSS is a custom post type? I’ve never thought of that approach, and would love to here what others have to say.

          • Justin Tadlock 12:22 am on March 1, 2015 Permalink | Log in to Reply

            I’d probably use a CPT myself if building this sort of plugin. It makes pretty good sense to me.

            Ryan is specifically talking about how the data is sanitized/validated from what I understand. How the data is stored is actually a separate matter.

    • Zulfikar Nore 12:48 am on February 11, 2015 Permalink | Log in to Reply

      Thanks for the example code Justin.

      Question: What would be the best method to escape the output?

      Zulf

      • Justin Tadlock 4:49 pm on February 11, 2015 Permalink | Log in to Reply

        Just hooking it to `wp_head()`? I’m not sure you’d want to escape it. If you’re allowing users to enter arbitrary code, you’ve given them a lot of rope to hang themselves with. Most of the escaping functions would probably break some CSS.

        I’ll give it some thought.

    • webriti 5:24 am on February 11, 2015 Permalink | Log in to Reply

      What’s the perfect implementation in case of option framework

      • Justin Tadlock 4:39 pm on February 11, 2015 Permalink | Log in to Reply

        The Customizer has been around for two years or so. It’s time to move on to using the core-supported standard for theme options. That’s my official recommendation.

        It’d really depend on which option framework you’re using, but you’d have to use the required capability check and one of the functions mentioned above to sanitize.

        • KProvance 12:08 am on February 20, 2015 Permalink | Log in to Reply

          Why? It’s sucks, and I’m not just saying that out of bias. It’s kludgy and incredibly difficult to code for. I get that the core devs think it’s the bee’s knees…and in some ways (live preview) it is, but as far as dev friendly, and easier for the user to understand. No. And I’ve been designing software for decades, so I know just a bit of where I speak.

          • Justin Tadlock 3:01 am on February 20, 2015 Permalink | Log in to Reply

            Standardization is probably the best answer as to why. I imagine TRT will one day require this for all theme settings. That’s the general direction we’re heading.

            If you think it’s not good enough yet, get involved. Create tickets on Trac. Post wireframes. Discussion on the TRT blog isn’t going to help. Involvement with core might.

    • nikeo 8:21 am on February 11, 2015 Permalink | Log in to Reply

      Thanks for sharing this @greenshady

    • nvartolomei 8:55 am on February 11, 2015 Permalink | Log in to Reply

      Why not leave this for children themes? If user knows what css is and what to do with it i think he can manage to understand what children theme is and how to customize it.

      Why do you need this in customizer?

      • Justin Tadlock 4:35 pm on February 11, 2015 Permalink | Log in to Reply

        I agree with that. Plus, a user could install a plugin if they really didn’t want to do a child theme.

        I also think there’s a case to be made for users who are just “getting their feet wet” and trying out CSS for the first time.

    • Stanko Metodiev 11:53 am on February 11, 2015 Permalink | Log in to Reply

      I’m glad that you’ve decided to not to restrict this option to the plugin territory. I my opinion if the user is administrator he/she can break his/her site in several other ways, so he/she should be able to add Custom CSS. Thanks for the code example, too! :)

    • Dovy Paukstys 10:25 pm on February 19, 2015 Permalink | Log in to Reply

      Disappointing I did all this work to modify my framework when you hadn’t reached a decision. Please don’t post on framework issue trackers before you decide your requirements. Reverting to what we had before so we can pass theme-check again.

      • Justin Tadlock 3:13 am on February 20, 2015 Permalink | Log in to Reply

        You were contacted after TRT had reached a decision. We just happened to later change that decision because I personally brought it back up for discussion on behalf of you and others with similar theme options. You were also given an opportunity to discuss this at the time here on the previous post.

        If you don’t want TRT to post on your issue tracker, you should not have it open to the public or allow people from our team to post on it. We will always take any steps we feel necessary to help theme authors and users, particularly when there’s a question of security involved.

  • Jose Castaneda 7:38 pm on February 10, 2015 Permalink |  

    Weekly Chat notes 

    Slack link: https://wordpress.slack.com/archives/themereview/p1423591238002102

    Topic: quality

    • example comments post ( @jcastaneda )
    • handbook examples

    Sub-topic: child themes

    • Thinking ahead:
      • Parent theme updated in 2 years
      • when to suggest fork

    Re: Custom CSS

    • theme territory, security is key
    • use proper capability check and sanitize
    • CSSTidy for validation
    • SECURITY
     
  • Jose Castaneda 10:40 pm on February 9, 2015 Permalink |
    Tags:   

    Chat Agenda

    Topic: Review Quality

    • brought up by @emiluzelac: https://wordpress.slack.com/archives/themereview/p1422989633001347
    • comments are valuable!

    Sub-topic: Child themes

    • brought up by @cais: https://wordpress.slack.com/archives/themereview/p1422989532001337
    • missing rules/requirements

    Looking forward to seeing ya’ll there! I’ll do a little housekeeping about ten minutes prior to the scheduled time. Same Slack channel ( #themereview ) and time 1800UTC.

     
    • Edward Caissie 2:13 am on February 10, 2015 Permalink | Log in to Reply

      I expect the usual schedule conflict(s) but will try to be at least available for the Child-Themes discussion.

    • Ulrich 6:00 am on February 10, 2015 Permalink | Log in to Reply

      I would love to have some input “How to review complex Themes and frameworks?”

      I have two themes that I am helping review/reviewing that are quite complex with a numerous classes and files. There are some complex settings frameworks too.

      With the large number of code it takes longer to do the review and it is more difficult. One needs to spend time understanding the logic of the code. What gets to me is that it takes a lot longer to do the review and then I am still a bit unsure if I have missed something.

      The goal of the discussion is to come up with a few suggestions how to review large chunk of code.

      • Carolina Nymark 11:16 am on February 10, 2015 Permalink | Log in to Reply

        I’m bitter atm hehe but “don’t submit code you don’t understand” should be a requirement…

        • Ulrich 12:12 pm on February 10, 2015 Permalink | Log in to Reply

          I understand, the two themes that I mentioned were most likely built from scratch and the theme author understands the code.

          Can we really restrict the complexity of the themes as we are unable to review them?

          How many hours should we spend reviewing a theme?

      • Jose Castaneda 1:03 am on February 14, 2015 Permalink | Log in to Reply

        I like it. The 17th is our next meeting. Can we get a list of frameworks to see how many there are. That way we can at least get an idea of what options theme authors have and see how we and others can tackle such frameworks.

        • Justin Tadlock 5:58 pm on February 14, 2015 Permalink | Log in to Reply

          I tend to think settings-page type frameworks are the most complex because they’re an additional layer on top of two existing core APIs that, while not perfect, get the job done and are fairly easy to use. Part of me really wishes that we’d force all theme authors to simply build their options via the customizer. Or, require PHPDoc and inline comments. :)

          I think a lot of the problem is that some theme authors never learn the WordPress core way of doing things. Instead, they learn the framework first. Most frameworks are created to solve some sort of problem, but without a true understanding of the problem, theme authors cannot figure out whether the framework actually does things that are suitable for a particular theme.

          I see this a lot with my own framework. People try to learn how to utilize it without understanding the issues that it fixes. Truth be told, many of them don’t need it for a particular project.

          If you can’t tell me how a particular setting you’ve created is being sanitized, for example, you probably need to spend some time learning the core APIs first. I see this all the time.

          Sorry, that turned into a mini rant somehow. All I really came here to do was list some of those frameworks. Here’s three off the top of my head:

          • Hybrid Core (this one’s mine)
          • Options Framework
          • Redux
          • Jose Castaneda 6:09 am on February 15, 2015 Permalink | Log in to Reply

            If you can’t tell me how a particular setting you’ve created is being sanitized, for example, you probably need to spend some time learning the core APIs first.

            Very well stated and I fully agree with that.

            A few I’ve seen:

            Keeping in mind that was also a quick search. I’ve seen one Option tree in the repo but can’t recall what theme used it.

          • Courtney Ivey 9:02 pm on February 16, 2015 Permalink | Log in to Reply

            > Or, require PHPDoc and inline comments.

            This. Right here.

    • Ulrich 12:19 pm on February 10, 2015 Permalink | Log in to Reply

      In the last meeting we came to the decision that we require themes to sanitize the CSS correctly and recommend them not to add the Custom CSS option.

      There are still a few open questions on the implementation. Justin gave a few suggestions: https://make.wordpress.org/themes/2015/02/03/theme-review-chat-notes/#comment-40876

      I tried to create a summary of all of the information.

      Sanitization – Require CSSTidy
      Escaping – What do we require here? Jetpack creates a file to display the CSS so
      Capability – Require use of `edit_themes`

      Possible Plugins:
      https://wordpress.org/plugins/reaktiv-css-builder/
      https://wordpress.org/plugins/jp-custom-css/
      https://github.com/jocastaneda/custom-csstidy

      Possible Frameworks:
      https://github.com/ReduxFramework/wp-custom-css-validation

    • Ulrich 1:52 pm on February 12, 2015 Permalink | Log in to Reply

      What do you think about creating a report for all of the themes that need a new reviewer?

      It should be possible to add a tag “needs-reviewer” manually. Here is a screenshot how it can be done.

      We could create a report with all the theme listed with the tag “needs-reviewer”.

      • Sakin Shrestha 7:35 am on February 19, 2015 Permalink | Log in to Reply

        @Ulrich: We need this soon

      • Tammie 11:15 am on February 19, 2015 Permalink | Log in to Reply

        I’m kind of not for this. Why? Because we already have queues and having another queue that simply says ‘needs reviewer’ when we already have a lot of tags and queues.. it’s adding to what we’re already trying to reduce. We got rid of the multi-queues because they didn’t work. It was a horrible first user experience and didn’t even really work for most longer term users.

        I first think rather than throwing a report we need to think what are we trying to solve that our existing reports don’t meet. I don’t see how this fulfills anything our existing ones can’t or don’t do. Another tag is another hurdle, another thing to do – we need less not more things. We already can shout out in Slack. Having a way that themers can shout ‘look at me’ and get above in the queue – that’s not awesome.

        If the intention (and the intention is a little foggy) is to flag that something needs attention.. well we already have that in slack. A report that has ability to shout more, well it’s just going to have more shouting.

        I’d be strongly against this report. To me, it is just another way of adding complication, of adding a way to shout more. Of adding something that we don’t need and a complexity that doesn’t solve the problem, just adds to it.

        Now, in strongly disagreeing with this I also recognise that there is an issue of ticket abandonment. There is also an issue of tickets being left without reviews. I think looking to solve those without adding yet another shouting mechanism or report is key.

      • Ulrich 9:11 pm on February 23, 2015 Permalink | Log in to Reply

    • Emil Uzelac 9:23 pm on February 23, 2015 Permalink | Log in to Reply

      The intention behind this is great, however it doesn’t solve the issue itself.

      Reassigning is actually lack of free time, that’s all.

      What could work and I am not sure if this can be done or not is to have some logic behind the button, where if there is no response from the reviewer within 7 days it can be automatically reassigned to anyone that requests a ticket from the button.

      I believe that this way we could solve a bunch with a single click 😉

  • Emil Uzelac 9:52 pm on February 7, 2015 Permalink |
    Tags:   

    New TRT Admin: Ulrich Pogson 

    Welcome Ulrich Pogson (@grapplerulrich ) to Team Review Admins!

    Ulrich started with WordPress back in 2011. He is a student, sportsman and WordPress developer with interests in responsive design and internationalization and that is not all: he is also Meetup Bern organizer and WordCamp Switzerland co-organizer.

    Thank you for your dedication Ulrich.

     
  • Konstantin Obenland 11:43 pm on February 5, 2015 Permalink |
    Tags: Theme Directory   

    Test the new Theme Directory 

    The new Theme Directory is in its Beta phase and I want to open it up to bug reports form the community. It would be great if you head over to the following URL and give it a spin:

    https://wordpress.org/themesnew/

    If you find anything wonky or out of the ordinary, please feel free to file a ticket over at meta.trac, and assign it to the Theme Directory component.

    Thanks so much for your help!

     
    • Emil Uzelac 11:59 pm on February 5, 2015 Permalink | Log in to Reply

      Awesome job, thanks!

    • Shaped Pixels 12:12 am on February 6, 2015 Permalink | Log in to Reply

      Definitely an improvement from what it is (was). Will there be a “Support” button added when this goes live?

    • jancbeck 12:15 am on February 6, 2015 Permalink | Log in to Reply

      Love it!

    • usability.idealist 12:29 am on February 6, 2015 Permalink | Log in to Reply

      *cough* like the horrible loading time for PNG files, that are each up to 600 kb per file?
      you do known that pngquant and imagemagick ARE real life solutions for that kind of issue, dont ya?

      cu, w0lf.

      • Emil Uzelac 12:35 am on February 6, 2015 Permalink | Log in to Reply

        Loads instantly for me :)

        • usability.idealist 12:56 am on February 6, 2015 Permalink | Log in to Reply

          So, you got a 16+k connection? I don’t. Neither has everybody in the rest of the world (not to forget about mobile connections, too).

          Long loading times for a page ARE BAD. Were bad 10 years ago and are STILL BAD in 2015.

          cu, w0lf.

          • Ipstenu (Mika Epstein) 1:47 am on February 6, 2015 Permalink | Log in to Reply

            Based on speedcheck, I’m on a 100k connection (hey, finally that worked!) so yeah, lots of people on high speed for this will cause problems.

            That said, I think the screenshots being a PNG are at the decision of the theme dev. They are for plugins. You can use either PNG, JPG, or GIFs as you see fit. Not sure if that’s a thing that can be fixed … @obenland ?

          • Emil Uzelac 8:57 pm on February 9, 2015 Permalink | Log in to Reply

            I got super fast connection, that’s probably it 😉

      • Dion Hulse 3:46 am on February 6, 2015 Permalink | Log in to Reply

        Yes, currently we’re loading the same theme thumbnails as we do for the existing directory, the CDN setup we’re using for these screenshots doesn’t currently allow for much configuration. If theme authors were to compress their thumbnails more, it’d certainly help with Theme ZIP total size and the weight of these pages…

        This does mean that the page load is rather heavy for slower connections, but is no worse than the pageload within WordPress itself, and only worse than the old directory in that this loads more than 10 items by default and allows a river, which can quickly increase the MB size of loaded images.

    • Emil Uzelac 12:34 am on February 6, 2015 Permalink | Log in to Reply

      Not out of the ordinary, but something to think about: How about instead of “Downloads Per Day” to say “Daily Downloads” instead.

    • Piet Bos 12:59 am on February 6, 2015 Permalink | Log in to Reply

      Looks like a huge improvement :)

      I filed a little bug that I experienced when attempting to go back after a preview. https://meta.trac.wordpress.org/ticket/843#ticket

    • Ipstenu (Mika Epstein) 1:57 am on February 6, 2015 Permalink | Log in to Reply

      Screen focus ‘jumps’ if your browser is smaller than the screenshot + download button

      https://meta.trac.wordpress.org/ticket/847

    • Jose Castaneda 3:59 am on February 6, 2015 Permalink | Log in to Reply

      Looks good! Great job so far!

    • Aaron Jorbin 4:38 am on February 6, 2015 Permalink | Log in to Reply

      Nice work. I think the fact that this aligning with the theme browser inside core will improve the experience for users.

      One thing that I noticed quickly was that when tab navigating, inside the model none of the links in the right side of the screen are focusable.

      Also, since infinite scroll doesn’t kick in when tab navigating, perhaps at the bottom of the list there should be a link to “load more”?

    • Chandra M 5:55 am on February 6, 2015 Permalink | Log in to Reply

      Great fresh look and much more aligned with .com! Awesome work!

    • Frumph 7:28 am on February 6, 2015 Permalink | Log in to Reply

      This looks really well thought out and pleasing to the eye. Very nice.

    • Thomas from ThemeZee 7:42 am on February 6, 2015 Permalink | Log in to Reply

      The new design is really great and a huge improvement for browsing themes.

      Are there any plans to allow theme developers to add Screenshots, FAQ and Changelog tabs similiar to the plugin directory?

      In my opinion it is great that the theme pages of the new design only display reduced information (description, reviews, stats) about the theme on a single page, that helps to get a first impression about it real quickly.

      However, it would be nice if there were some links to more detailed information. Screenshots to see how the theme handles theme options and a changelog to see what has changed in the recent update for example. As far as I can see the new design is used in addition and includes already hyperlinks back to the old design for Reviews and Support.

      • Konstantin Obenland 6:10 pm on February 6, 2015 Permalink | Log in to Reply

        Are there any plans to allow theme developers to add Screenshots, FAQ and Changelog tabs similiar to the plugin directory?

        There are long-term plans for those features, yes. :)

        it would be nice if there were some links to more detailed information.

        I agree. In the meantime I’d encourage theme authors to use the Theme URI for that.

        • Thomas from ThemeZee 8:24 pm on February 6, 2015 Permalink | Log in to Reply

          That is really awesome news to hear :) Can’t wait to see this happen.

        • Justin Tadlock 4:38 pm on March 4, 2015 Permalink | Log in to Reply

          The Theme URI is no longer useful since it’s no longer shown on the theme page. I love a lot of things about the new design, but users can’t figure out how to get to my theme page on my site for additional information about the theme.

    • Gary Jones 10:54 am on February 6, 2015 Permalink | Log in to Reply

      Are the filters working (when there are multiple features required)? See https://www.dropbox.com/s/3kc2d0kw34qbhlk/Screenshot%202015-02-06%2010.51.26.png?dl=0 – that’s got three feature filters enabled, which on the current directory gives a total of 20, and not 1686!

    • Gary Jones 11:00 am on February 6, 2015 Permalink | Log in to Reply

      It’s good that opening the modal focuses on the Download button, but when tabbing twice to get to the navigation arrows (top left of the modal), and hitting enter, the focus again jumps back to the Download button, meaning navigating twice involves hitting tab several times again. I suggest not automatically focusing the Download button when the modal was already open.

    • Gary Jones 11:10 am on February 6, 2015 Permalink | Log in to Reply

      Does there need to be better support for when JavaScript is disabled? The /themesnews/commercial page looks and loads fine without JS, but the /themesnew page just has a non-working bar at the top, and then an empty page. I would expect at least the first page of thumbnails (linked to single theme details page) with standard pagination, and *then* if JS is enabled, it can hijack the links with modals, infinite scrolling etc.

      • Konstantin Obenland 6:15 pm on February 6, 2015 Permalink | Log in to Reply

        Does there need to be better support for when JavaScript is disabled

        Yes, and there will be. This is a matter of having themes available that WP_Query can find, currently they’re still not synced over.

        If you inspect the source, there is an API output from the server that we use to test, and it’s hidden by an inline style for now.

    • Sakin Shrestha 3:24 pm on February 6, 2015 Permalink | Log in to Reply

      Looks really nice. Cheers :)

    • Simon Prosser 5:31 pm on February 6, 2015 Permalink | Log in to Reply

      Looks great. Shame to see the bundled themes are still in the most popular list, its like MS bragging that notepad is the most installed text editor, yes it is because you have no choice 😉

    • LittleBigThings 8:45 pm on February 6, 2015 Permalink | Log in to Reply

      Nice, cool WordPress experience… Cheers & thanks! :-)

    • Christine Rondeau 9:29 pm on February 6, 2015 Permalink | Log in to Reply

      Love it. Looks really good.

    • BriniA 10:04 pm on February 6, 2015 Permalink | Log in to Reply

      I like the clean and fresh style. I think the Download button should be always shown not only when the user hovers over a theme. While you scroll to very bottom and it’s loading, it would be great to show a loading animation and maybe a Load more button.

    • S M Hasibul Islam 5:01 am on February 7, 2015 Permalink | Log in to Reply

      Looks really good.

    • BFR 9:14 am on March 4, 2015 Permalink | Log in to Reply

      I didn’t really know whether to open a ticket for this on

      So, I am just posting it here. If anyone from the UI/UX team think that my arguments/suggestions here are valid, then they may create a ticket for the same.

      Pulled it from https://wordpress.org/support/topic/theme-directory-ui-update-feedback, since no team member or moderator has replied yet. Plus I didn’t want to really come here and write this, but the more I came across the new theme directory page the more I felt tempted to just let you guys know that a lot of things have been done wrong, and will impact the UX for folks here very badly…

      While all seem to be quite content with the new UI, I am not really happy with it. A lot of things (functionalities or the UX) have been changed without much thought. Let me explain.

      1. The author themepage url is missing
      2. There should no pop up kind of thing. It adds to load time.
      3. The current theme version is missing
      4. There should be no scroll (as suggested by jwhittaker99 and BFR)
      5. The preview url should directly show the preview and not take to the theme page first
      6. Theme preview should be full screen
      7. Theme previews should have a unique url for itself
      8. When the theme was last updated (now resolved)

      2. There should no pop up kind of thing. It adds to load time.

      Before: The old theme page would load in 1 second.
      Now: The new UI has a lightbox pop-up (if I am naming it right) and so, the page takes 1 second to load and that box takes another second to load, which is not at all cool.

      3. The current theme version is missing

      Before: Oh, that’s the current version. Fine. Done.
      Now: Go all the way to the download button to check the current theme version. You just added an extra step.

      6. Theme preview should be full screen

      When I click on preview, it shows the theme preview. But the left sidebar covers 20% (or so) of the screen. I then have to click on the arrow at the bottom. Now comparing this to what it was before:

      Aim: Preview a theme

      Before:
      Step 1: Open the url.
      Step 2: Either click on preview or right click and select open in new tab. Easy peasy.
      Now:
      Step 1: Open the url. Wait (for the pop up box to load).
      Step 2: Click on preview.
      Step 3: Click on the arrow to see full screen preview.

      You just added an extra step, again! Which would not really seem a problem to folks who come here sometimes, but to us, who surf the theme dir everyday, this is not at all cool. You spoiled my UX.

      7. Theme previews should have a unique url for itself

      I am watching a theme preview right now. When I duplicate this page in my browser (Google Chrome in this case) I don’t get the theme preview. I get the theme page instead. Now I need to repeat the steps to see the preview.

      Previously I had a link to share. I could easily direct my friends who don’t know WP much to a theme’s preview url. Now there’s no preview url. And I hope you guys aren’t going to kill the existing urls (previous wordpress theme preview links on wp-themes.com)

      I am sincerely sorry if any of this sounded rude. I am just giving honest feedback.

      Unhappy with this update. :(

  • Jose Castaneda 7:04 pm on February 3, 2015 Permalink |  

    Theme Review Chat Notes 

    Topic: Custom CSS

    • Discourage usage in favor of plugin
    • If used, must take security, permissions, escaping into account ( ping somebody capable of doing such review )
    • Security is biggest concern
    • Need good examples of _doing_it_right_

    Sub-topic: requirement examples

    • Add items to https://make.wordpress.org/themes/handbook/review/required/explanations-and-examples
    • Encourage commenting but will have a weekly after meeting for examples

    Next Week’s topic: Review Quality

    • Security
    • no-comment-approvals

    Link to meeting in Slack: https://wordpress.slack.com/archives/themereview/p1422986425001223

     
    • Jose Castaneda 7:54 pm on February 3, 2015 Permalink | Log in to Reply

      Addendum to next week’s topic(s):
      Child themes: https://wordpress.slack.com/archives/themereview/p1422990661001394 brought up by @cais

    • Tammie 1:41 am on February 4, 2015 Permalink | Log in to Reply

      Thanks for the update and leading the meeting.

    • Ulrich 7:22 pm on February 4, 2015 Permalink | Log in to Reply

      I am looking into the “Custom CSS” at the moment. Proper sanitization would require the use of CSSTiday? Do we require something for escaping the CSS before output? Ping @greenshady

      Can this be added to the requirements/documentation please?

      • Justin Tadlock 8:17 pm on February 4, 2015 Permalink | Log in to Reply

        I’d really like to see if we can get @nacin and/or @otto42 to weigh in on what’s proper. Or, maybe someone from the Jetpack team who’s worked on this feature. I’m not really in favor of themes doing this, but if we’re allowing it, I’d like to bring in someone I trust to tell us the best method.

        To sanitize, CSSTidy is probably the best place to start. I’d really look into what Jetpack is doing and study their code. If someone could break that down, it’d be nice.

        The correct capability would probably be edit_themes. That seems to make the most sense to me since anyone with that cap can directly edit theme files.

        • George Stephanis 8:50 pm on February 4, 2015 Permalink | Log in to Reply

          Yeah, it’s super tricky to do right and will necessitate a large sanitization library. The Custom CSS code in Jetpack itself is about 600k+ — but that’s including the Sass and Less preprocessors that we bundle. CSSTidy by itself looks to be about 160k.

          Honestly, my recommendation would be to tell themers to suggest an external plugin. Norcross has one as well — https://wordpress.org/plugins/reaktiv-css-builder/ — and there’s a version of the Jetpack one that’s been spun off into its own plugin (yay GPL!) here: https://wordpress.org/plugins/jp-custom-css/ if they don’t want to recommend Jetpack itself.

      • Andrew Nacin 5:22 am on February 5, 2015 Permalink | Log in to Reply

        Despite CSS not being JavaScript, it is just as dangerous. There is no good way to escape arbitrary CSS. There is no good way to sanitize arbitrary CSS. There is no way to do this correctly. A theme should not do this, because libraries can barely do this correctly. They’re patching the CSS sanitization libraries running on WordPress.com all the time.

        This is plugin material, even before weighing all of the additional security concerns.

    • Dovy Paukstys 8:54 pm on February 5, 2015 Permalink | Log in to Reply

      Given the response here, and the query of Justin Tadlock, Redux Framework has integrated the CSS sterilization found in JetPack.

      I’ve also isolated the required code that you could pass to any developers to ensure they comply with these requirements: https://github.com/ReduxFramework/wp-custom-css-validation

      • Dovy Paukstys 9:06 pm on February 5, 2015 Permalink | Log in to Reply

        On a side note, I don’t know why this isn’t just added to WordPress core. It’s only 195kb in all and would be a very useful function. I’ll gladly contribute it if you think it would be accepted. 😉

      • Justin Tadlock 9:11 pm on February 5, 2015 Permalink | Log in to Reply

        @dovyp Thanks for taking the time to flesh out the code in your framework for those theme authors who are using it. That’s definitely a step forward.

    • Kadence Themes 3:18 am on February 6, 2015 Permalink | Log in to Reply

      Hey, I’ll add in, I am for security but strongly disagree that because it’s not easy to do it should be put on plugin developers. I really don’t understand how css needing to be properly sanitized turns into this should only be in plugins? What are the hundreds of themes that have css boxes supposed to do? remove and have all there users lose there css? Come on, we should be promoting safe ways to do this not pushing it off the review team.

      If we are for making sites more secure then themes should be able to lead the way. I took a look around what plugins are available for custom css and it’s not like they are all doing it so well that theme developer should just hand in the towel.

      I could argue all day that I think themes should have the option to add this. But I think that has to be a different point. The point is all css boxes need security and there should be an accepted way to do that. If jetpack has the answer then why can’t we promote the use of jetpack as a plugin or it’s code as a solution?

      • Justin Tadlock 5:44 am on February 6, 2015 Permalink | Log in to Reply

        I am for security but strongly disagree that because it’s not easy to do it should be put on plugin developers. I really don’t understand how css needing to be properly sanitized turns into this should only be in plugins?

        I just want to point out that no one is arguing this at all. The security and plugin territory issues are two separate and unrelated issues.

c
compose new post
j
next post/next comment
k
previous post/previous comment
r
reply
e
edit
o
show/hide comments
t
go to top
l
go to login
h
show/hide help
shift + esc
cancel