Using an EditorConfig helps developers define and maintain consistent coding styles between different editors and IDEs. EditorConfig files are easily readable and they work nicely with version control systems. This is especially useful when there are many of us pushing to one staging branch and we want to reduce as many conflicts as possible.

http://editorconfig.org/#download

I have been using these in many projects already and have only had positive experiences. Today, I just noticed it is not included in our best practices. Have you been using .editorconfig? Do you think something like my first paragraph could be a useful entry?

Splitting off the discussions from #101 into separate issues:

Responsive images can be considered in two places: their use in the theme (static images and featured post images, for example), and embedded images inside post content.

One way that I've dealt with images (for the post featured images) can be seen here:
https://gist.github.com/zrothauser/a8e27f68d4ccdfb9881c
which would result in a syntax of: https://gist.github.com/zrothauser/a58d1d3cf4cc3f285a85

One potential core feature (currently in progress) to consider:
https://make.wordpress.org/core/2015/08/25/responsive-image-support-update/

Regarding the JavaScript section for Event Delegation; could its purpose be clarified?

If we are listening for a click event on an <li> element, is it because we intend to do something with that element? Something outside normal browser behavior (i.e. what <a> or :hover already provide us)? For instance, could it be an item in a shopping cart, or a parent in a drop-down menu?

If we plan to do something with the <li> element, would it be useful to provide a safer code example? The current example checks for the existence of event.target, but what if the actual target is a child of the <li>? This scenario would seem very likely. While more verbose, a safer example might be:

document.getElementById( 'menu' ).addEventListener( 'click', function( event ) {
    var currentTarget = event.currentTarget;
    var target = event.target;

    if ( currentTarget && target ) {
        while ( currentTarget.contains( target ) ) {
            if ( target.nodeName === 'LI' ) {
                // Do stuff with target!
            } else {
                target = target.parentNode;
            }
        }
    }
});

A more loose example could be:

document.getElementById( 'menu' ).addEventListener( 'click', function( event ) {
    var target = event.target && event.target.closest( 'li' );

    if ( target ) {
        // Do stuff!
    }
});

While straight to the point, the above example uses closest, which to support Safari and older IE would require a 270B polyfill.

Thoughts?

Currently, the BASE_PATH always points to the live site. This leads to some confusion when working on the site locally ("WHY ARENT MY STYLES UPDATING!?").

Not sure yet what the best approach is, but will start looking into it when I have a little more free time.

We talk about no_found_rows, but in reality that only removes one query out of the five that WP_Query runs in its typical configuration. We should also document: update_post_term_cache, update_post_meta_cache, and 'fields' => 'ids'.

I feel we should mention using PHP as an alternative method instead of using post__not_in within a query. VIP recommend this in most cases.

Happy to submit a PR.

We should outline best practices for web fonts.

1. When and how to wire up self-hosted fonts.
2. How to best wire up font stacks.
3. How to manage page weight, async loading, fallbacks.

I’ll chime in with my own opinions as time permits, but I’m more interested in knowing how others approach this. I think it could be very valuable for everyone as web fonts are quite common.

I've been taking the course: https://www.pluralsight.com/courses/javascript-good-parts and it appears that there is a tool, similar to PHP Code Sniffer called JS Lint.

Is this something we would want to encourage in our best practices?

Not sure if this should be for just secondary or secondary and tertiary sections.

I believe https://github.com/Automattic/batcache/blob/master/advanced-cache.php#L384 shows that batcache does cache full pages with query arguements, it just saves different query strings as unique pages (which is why you can defeat the cache and get a fresh page by adding ?new=yes at the end of a URL).

I believe this is in conflict with line 190 in PHP.md:

"As Batcache does not cache calls for URLs with query strings or logged-in users (based on WordPress login cookies), you will need to make sure that your application design uses pretty URLs to really benefit from this caching layer."

I'd like it if someone who actually does PHP development confirmed this for me.

Also worth noting: the version of batcache on Github is not the version we use - it is altered with many updates from Automattic based on WordPress.com needs. We use the .org version https://wordpress.org/plugins/batcache/installation/

For our projects Sage and Soil, we use add_theme_support() to decouple the theme and the plugin. Removing the plugin (Soil) does not result in any errors, and changing the theme (Sage) does not result in any errors. This is another approach for decoupling code using standard WordPress paradigms, and I think it is worth mentioning.

Has any thought been given towards adding a section outlining some general recommended wp-config settings?

The thought occurred to me as I read the following article by Brad Touesnard outlining the potential space-saving benefits of the IMAGE_EDIT_OVERWRITE constant, but didn't see a suitable place in the Best Practices where this kind of solution would fit: https://deliciousbrains.com/undefined-define-image_edit_overwrite/

The use of the tabindex attribute should not be considered the primary approach to keyboard accessibility. As Joe Dolson points out in this article it often does more harm than good. tl;dr: Planning a logical document structure prevents having to use tabindex all over the place, allowing you to use it only when needed.

Title says it all.

Image handling is a subject that has become more complex recently, as responsive images are now more widely supported and are encouraged to support newer retina devices and to save bandwidth for mobile users. I think it would be worthwhile to have a section under Markup that deals with how we should implement images on sites.

I have a very rough draft here on what this section could include, and I'd like to get some more input on this before creating a pull request:

Responsive Images

10up recommends the use of SVG (with an icon font fallback as needed) to serve high-resolution assets at the smallest file size possible. When using SVG you should always provide a fallback such as a PNG image for browsers that do not support vector graphics. [this sentence was moved from Progressive Enhancement section]

For user-uploaded content images, responsive images should be used in order to serve both high-resolution images to high-density devices and to avoid sending unneeded data to mobile devices. To follow the latest standards, responsive images should be served with srcset rather than <picture>, as long as the same image is being served at multiple sizes. If different images are being served at different resolutions, then <picture> should be used.

At this time, not even all current browsers don't support the complete srcset syntax, so the Picturefill polyfill is recommended to support the standard syntax. This is an exception to our guideline of using only the polyfills for features that are functionally critical to the site.

An example is shown below:
https://gist.github.com/zrothauser/a8e27f68d4ccdfb9881c

which would result in a syntax of:
https://gist.github.com/zrothauser/a58d1d3cf4cc3f285a85

Lazy Loading

In order to save on bandwidth, lazy loading images is recommended for archive pages or any page with a series of images.
[elaborate on this? or do we need this section?]

Image Sizes

The fewest image sizes needed should be defined with add_image_size to avoid adding too much time to the upload process and to avoid using too much server space. However, most images should include a doubled size (e.g., 'home-thumbnail-double' to go with 'home-thumbnail') to supply a high-density image for srcset images.

[do we want to recommend Photon from Jetpack on non-VIP sites? It's included on VIP]

[are there any other subsections we could add to this?]

@nicoladj77 brought up an excellent point this morning in a discussion on a current project: is it worth adding a section that advocates using i18n functions by default and never hard-coding a string in a theme?

Example:

<h1><?php 
  esc_html( 
    sprintf( __( 'Search results for "%s"', 'text-domain' ), get_search_query() ) 
  ); 
?></h1>

vs.

<h1>Search results for "<?php the_search_query(); ?>"</h1>

Internationalization by default forces us to think about what content we're hard-coding, makes the sites ready to be translated at a moments' notice and, especially given core's recent (re-)focus on i18n, feels like a strong recommendation. Being translatable is also a requirement for themes submitted to WordPress.org, so there's some degree of precedent.

Had a quick peek at a big cross-section of our codebase, and after removing all preceeding whitespace, out of those that begin with for, if, while:
cat loops-conditionals|grep :|wc -l ~= 100
cat loops-conditionals|grep {|wc -l ~= 800

Does it make sense to specify alternate syntax for control structures?

We've discovered inconsistencies with how namespaces are applied on some projects. Now that 10up engineers are adopting namespaces more widely in their projects, we should be providing more specific guidelines on how to use them in our projects.

A pull request will be coming after some thought on my end, but I wanted to open an issue to get my thoughts together. Any input is welcome.

We do this to logically organize our code and to prevent collisions in the global namespace

What does this mean to someone with no experience with namespaces? What's the "global namespace"? How do things "collide" there? Was anybody hurt?

Let's explain this more clearly, perhaps with a link to an external resource with a good explanation for beginners.

namespace tenup\Utilities\API;

We need to set a standard namespace for 10up projects, and include general guidance for non-10up developers who want to learn from our docs. I propose we stick with this "tenup" namespace that's already in our best practices, although maybe changed to "Tenup" or "TenUp" for consistency with PSR-compliant projects out there.

From there, I propose the next level of the namespace be the client's name (i.e. "tenup\Tyrell-Corporation") followed by a particular site or product name ("tenup\Tyrell-Corporation\Nexus-6"). Consistent namespace naming on a project helps autocompletion in IDEs like phpStorm, making development more efficient, making engineers more productive and EMs happier. On projects where there is a "shared plugin" with common functionality for different sites, we can call that sub-namespace "Util" or "Common".

Theme code and plugins within a project can share a namespace. If there's a collision inside that namespace, it's a sign we probably have redundant code, not a prompt to create a new namespace. Also, adding more namespaces means more "use" declarations or long \paths\to\namespaces\in_function_calls() which makes code harder to read.

Would Markup’s Progressive Enhancement section for Polyfills (and perhaps Modernizr as well) be better positioned beneath JavaScript’s Only Load Libraries You Need section?

Not to be nit picky... Mark-up page says "Use colon syntax for PHP loops and conditionals so that it's easier to see when a certain loop ends within the block of markup." But all of the code examples on the PHP page used bracket syntax? Make up your mind 10up!

Right now, code samples are not readable on mobile. Lines are broken making code illegible.

Do we need/want to include a section that outlines our theory on how and when for transitions and animation?

This section would:
• Identify when to use transitions/animations, when to avoid them
• Performance concerns and optimizations for transitions & animations
• UX considerations, such as animation duration (shout out to material design)
• Accessibility concerns (e.g. photosensitive epilepsy, moving objects, readability, whatever)

In the section Optimize Readability it sates;

When mixing PHP and HTML together, indent PHP blocks to match the surrounding HTML code.

Does this mean the PHP blocks should be aligned the same as the HTML markup? Or, should the PHP blocks be indented, as they would be if it was purely HTML?

For example the last code block shows -

<ul>
<?php foreach( $things as $thing ) : ?>
<li><?php echo esc_html( $thing ); ?></li>
<?php endforeach; ?>
</ul>

But from reading the above statement I feel it should look similar to this instead -

<ul>
    <?php foreach( $things as $thing ) : ?>
        <li><?php echo esc_html( $thing ); ?></li>
    <?php endforeach; ?>
</ul>

Is the statement misleading, or am I missing something?

We have been talking about this in the Front-End Engineers room. We should not commit compiled/minified css or js files to the repository. We could be using hooks to run grunt and compile them in the server. That way we will see a lot less of conflicts when committing.

I think we should add a section about this and start putting it into practice.

Improve nonce documentation to include replay attacks, and an example of nonce tied to an action (as per code example) and a nonce tied to an action tied to a unique ID (as per trashing post example)

On the PHP page under the Effecient Database Queries section is this:

WP_Query actually calls get_posts(); calling get_posts() directly bypasses a number of filters. Not sure whether you need these things or not? You probably don't.

I find that very confusing. It is actually the other way around: the get_posts() function uses WP_Query. I assume that maybe you meant to refer to the WP_Query::get_posts() method.

As of WordPress 4.3.1 (and probably earlier) get_posts() sets 'no_found_rows'to true which is recommended by your best practices. See https://core.trac.wordpress.org/browser/tags/4.3.1/src/wp-includes/post.php#L1900

You can still pass in 'update_post_meta_cache' => false, 'update_post_term_cache' => false, and 'fields' => 'ids' as needed.

I'm finding that headings are bleeding together a bit for me. Adding some weight and a keyline on the pages' primary section headings (h3) might help.

I've got this done in my local branch, but I'd like to have some feedback before committing.

Thanks!

For instance, look at https://10up.github.io/Engineering-Best-Practices/markup/#examples

Was curious how your team handles changelogs for best practice? Thought this would be great to have in your best practices.

Similar to http://keepachangelog.com perhaps?

Splitting off the discussion from #101 into separate discussions:

Our current statement on SVGs is "When using SVG you should always provide a fallback such as a PNG image for browsers that do not support vector graphics."

@daveross and @jonathantneal both questioned the need for PNG fallbacks, and @jonathantneal provided a possible update to the SVG wording:

Use SVG images whenever possible. They are resolution independent, easily styleable with CSS, and often smaller and clearer than other image formats.

When using SVGs as an icon system, combine them into a single sprite sheet that allows individual images be displayed with an ID.

When using SVGs and supporting Internet Explorer 8, provide a fallback, such as a PNG image.

This topic came up internally recently and we thought it'd be worth pitching here as well: should we consider offering advice on how, when, and if to use (here|now)doc syntax?

Take this code as example:

$image_html = <<<HTML
<div class="entry-thumbnail">
    <a href="$permalink>
        <img src="$image_src" />
    </a>
</div>
HTML;
return $image_html;

Pro/Con

• Pro: Easy to read. (vs. ?><!-- some html here --><?php )
• Con: Doesn't allow for truly late escaping.
• Con: Increased chance of mistake. One typo could cripple the code below.
• ??

CC: @daveross, @zrothauser

A last modified date (possible in the footer) would be useful on every page.

I've gotten questions about Composer best practices when we write themes & plugins. My advice is to use a separate composer.json for each plugin/theme. Do we want to codify this as a best practice (vs a single composer.json for each project)?

Also, Chris Marslender found a way to keep Composer downloads out of Beanstalk repos and run Composer during deploys. Might be good to have him document that to keep our repos small.

Splitting off the discussions from #101 into separate issues:

Do we want to recommend lazy loading images on image-heavy pages?

@TheLastCicada mentioned in #101 that it can be "fraught with peril", see that discussion for more details.

@tddewey mentioned he was looking into using our Typekit account for the fonts in #14.

Based on a HipChat discussion, @helenhousandi was kind enough to add the 10up.github.io domain to our Typekit account. Assigning to myself to wrap up the task.

At the end of http://10up.github.io/Engineering-Best-Practices/structure/#modular-code:

"In these cases, the dependency, its version, and the reason for its inclusion in the project outside of a dependency manager."

I think this sentence should read:

"In these cases, the dependency, its version, and the reason for its inclusion in the project outside of a dependency manager should be documented."

Link should be updated from http://10up.github.io/devstandards ---> http://10up.github.io/Engineering-Best-Practices

From the current best practices:

Resets
Please use normalize.css.

Now that 10up has adopted sanitize.css, should we include it into our best practices?

The PHP security section currently says:

esc_js() ensures that whatever is returned is safe to be printed within a JavaScript string.

This is not completely accurate. The inline docs in core continue on to say:

It is intended to be used for inline JS (in a tag attribute, for example onclick="..."). Note that the strings have to be in single quotes.

Given that we should not be writing JS in this manner anymore, esc_js() should never be used. Escaping strings for JS use can be done using json_encode(), or better yet wp_json_encode() which does some sanity checking for non-UTF8 and handles different versions of PHP.

I've used Code Climate for several personal projects and something that is recommended frequently is using filter_input() rather than accessing $_POST and $_GET directly. I didn't see anything in our best practices, so this might be something we could include.

I just published best practices for commit messages in WordPress core, much of which is general commit message advice. This is mostly a placeholder issue so we add this to our own best practices at 10up, with some blog posts I plan on writing to go along with it.

Title says it all :)

We need a section to explain how we document our code.

Hi there,

Wasn't sure if me forking this repo would create a private fork or not, and this issue is so small that I decided it wouldn't hurt to just bring it up to someone who's already committing.

Basically, if you look at the code example in the section titled Cache Remote Requests, there's a line that looks like this:

wp_cache_set( 'prefix_other_blog_posts, $posts, '', HOUR_IN_SECONDS );

There's a closing single quote missing right after prefix_other_blog_posts.

Like I said, really small. If it is safe for me to fork this repo without exposing it to the world, let me know and I can send a pull request, otherwise this issue is in the file _includes/markdown/PHP.md on line 220.

Thanks,
Faison

On the heels of a discussion from today, I'd like to possibly add more discussion around some specific Grunt tools - in this case, specifically PostCSS.

Sass remains our preprocessor of choice, but PostCSS presents some interesting extensions that can be very useful. For example, Autoprefixer is no longer supported as a standalone Grunt plugin and now lives on as a PostCSS plugin.

To be clear: while PostCSS is a "preprocessor" in name, it will not replace Sass (certainly not yet, at least), which is what we should cover in our Best Practices. Some of the "how" and "when" around using PostCSS.

Opening this issue as a reminder to myself to take a stab at a PR, but also to open this up for further discussion from any interested folks.