Welcome to WP Chat! Overall, everything seems pretty solid. Like Ben, I am also a theme developer so I’ll just comment on your theme repository.
As a sidenote to what Ben said, it’s worth noting that translations have actually been unescaped in the default themes, with some interesting discussion on Trac as to why. Note that this is just about escaping translations because it can be argued that they can be “inherently trusted.” Everything else should always be validated, sanitized, and escaped.
Personally, I prefer to be “better safe than sorry” so I always escape them anyway, plus it’s required on WordPress.com where I am a theme author along with Ben. Just always good to know both sides of the “debate.”
In modern HTML/CSS, I can’t think of any reason to ever use <br>. If you really need a line break, it’s best to wrap it in a <span> or something and style with
display: block. That way you have flexibility to remove the line break with a responsive breakpoint. So I’d adjust this line.
In 404.php, I would remove the <br> and then wrap the sentence directly underneath the heading in <p> tags. You can always adjust spacing and such with CSS if necessary.
Looking at functions.php, it’s usually recommended to add theme support via a function that hooks into after_setup_theme. I suppose it’s not that big of a deal though.
Also, that’s an interesting way of combining all the function files. A quick search on GitHub shows that it’s a popular method.
I’d question two things though:
Is there a performance issue? I’d imagine referencing specific files would always be faster than searching through an entire directory with a wildcard character. But it’s hard to say for sure without benchmarking it.
Is there a security issue? What if a hacker somehow managed to sneak in a file into the /functions/ directory, which was then automatically fired on every page load? I’m just thinking out loud here and if some hacker had access to upload PHP files to your theme directory, this would probably be the least of your concerns anyway.
Speaking of after_setup_theme, instead of hardcoding the <title> tag in header.php, use add_theme_support( ‘title-tag’ ).
It looks like you’re using the Yoast SEO plugin which is probably taking over your title tag in any case, but just a good idea to switch this up and use core functionality when available.
Instead of hardcoding a favicon, you could use WordPress core’s “Site Icon” feature. More info: https://make.wordpress.org/core/2015/07/27/site-icon/
In template-contact.php, I noticed <div class=“contact-form-header”>. While this may be the only place this particular stye is used at this time, and I’m nitpicking a little bit here, but you should always keep modularity in mind. Is this a style that could be used elsewhere?
I’d probably use a class like “page-header” so it can be reused in a semantic way in multiple places, if necessary. “Semantic” meaning, it wouldn’t make sense to use “contact-form-header” on a page with no contact form, even though there’s nothing technically stopping you from using that class elsewhere.
Speaking of the contact form page, it looks like you’re running your own contact form functionality through the theme.
It’s totally cool to experiment with this on a personal site, but in any other situation, you can save time by simply adding one of the many polished free contact form plugins, like Ninja Forms, and embed that into a normal page template with a shortcode.
In template-about.php, I’d question the need for a separate page template.
Again, since it’s for your personal site, it doesn’t really matter that much, but think about how the template could be made more flexible.
For example, you could replace the hardcoded links in <div class=“links”> with a widget area. That way, you could technically make unlimited pages with different menus with something like Jetpack Widget Visibility.
Similar to the above point, I’m thinking more in a “public release” rather than a “personal site” mindset, so I wouldn’t necessarily act on this feedback. Just something to think about:
In partials/analytics.php where your GA code is included…
I know that I used to include my GA code at the theme level, and then wondered why my traffic flatlined after I switched themes…not remembering that I included my tracking code in the theme itself.
For a personal site, it’s fine just as long as you’re not as forgetful as me.
Personally, I’d get in the habit of including the code outside of the theme. Even a mu-plugin that hooked into wp_head would be preferable to mistakenly forgetting to move that code over to the new theme.
As a general note, familiarize yourself with the WordPress Coding Standards, particularly the PHP standards. I noticed some spaces and such were off.
These are things that don’t really affect the execution of the code itself, just something to keep in mind when working with other developers so the codebase can maintain a consistent style.
get_field('company') should be
get_field( 'company' ) …notice the extra spaces in between the parentheses.
As someone who works with ACF from time to time, I can tell you their documentation doesn’t always follow WordPress coding standards when it comes to style.
Speaking of mu-plugins, it might be a better idea to register your custom post types and taxonomies through that, rather than at the theme level.
That way, you wouldn’t have to worry about copying over the code to any new theme you switched to.
For a personal site, it’s more of an issue of personal preference. Just another to keep in mind.
Finally, always remember to capitalize the “P” in WordPress. Since you’re hardcoding the content here, for example, capital_P_dangit wouldn’t take effect.