automattic / theme-tools Goto Github PK

Noticed while re-submitting Pique:
https://themes.trac.wordpress.org/changeset?old_path=pique/1.2.4&new_path=pique/1.2.5

It looks like annotations for Headstart are being stored in the inc/headstart directory. Since these are .com specific, they should be removed for .org submission.

The WordPress.org version 1.2.4 of Goran was throwing the following error:

Cannot redeclare class Edin_Theme_Plugin_Enhancements in /srv/www/wordpress-trunk/wp-content/themes/edin/inc/plugin-enhancements.php on line 10

The issue appears to be related to both Goran and Edin including Edin's plugin-enhancement.php file, causing the class Edin_Them_Plugin_Enhancements twice.

We need a way to make sure the script 'knows' a theme is a child theme, and does not cause the class to be declared twice (possibly by adding a check to see if it's already declared, or by using require_once to add the file).

This file was missing when I generated, Laurel managed to using MAMP. Perhaps a vagrant issue? Seems odd.

It'd be good to update Gutenberg test data. It's been 8 month since the last update, and since then some blocks have been updated such as core/cover that now takes more blocks inside.

It might be best to have a post for each block rather than a post for a group. Each post should have instances of the block with different configurations such as alignment and sizes of inner blocks. For example, Cover with all the headings, with paragraphs with different font sizes and alignments etc.

While testing Hew (which doesn't have dependencies, although we thought it did) I noticed that if there aren't any dependencies in the theme, the script will throw a PHP warning on the admin panel.

Notice: Undefined variable: dependencies in /Users/sarah/Sites/reviewbox/wp-content/themes/hew-gen/inc/plugin-enhancements.php on line 169

Ideally we wouldn't be adding this script to themes that don't need it, but it's probably best to fix that just-in-case.

The instructions on how to use this are unclear and it seems that Automattic's very own theme developers have difficulty using it correctly.
They only go as far as to require the PHP script.

The plugin-enhancements.js file however takes care of remembering a user's decision to clicking the warning message away.

As an example, the Pique theme by Automattic for some reason does not include the js file and therefore a user can keep clicking the warning (in a child theme), it will always come back.

Here's the error I'm getting:

Fatal error: Uncaught exception 'UnexpectedValueException' with message 'RecursiveDirectoryIterator::__construct(dyad-wpcom/inc/headstart/): failed to open dir: No such file or directory' in /Users/Ola/Web/theme-tools/org-submitter/make-zip.php:64
Stack trace:
#0 /Users/Ola/Web/theme-tools/org-submitter/make-zip.php(64): RecursiveDirectoryIterator->__construct('dyad-wpcom/inc/...', 4096)
#1 /Users/Ola/Web/theme-tools/org-submitter/make-zip.php(304): delete_directory('dyad-wpcom/inc/...')
#2 {main}
  thrown in /Users/Ola/Web/theme-tools/org-submitter/make-zip.php on line 64

It looks like this is because we're trying to remove the inc/headstart directory, without checking if it exists first, and many themes don't have it yet. PR coming shortly.

When Jetpack is active but the required module isn't, it would be nice to have a link that actually activates the module right there in the notice, or at least a link that takes them to the Jetpack settings page.

This came up while investigating a recent issue (#28): it would be great if the Jetpack Dependancy script could 'figure out' what's required from both the parent and child theme, to be displayed in the message on the child theme.

It looks like the only parent/child theme pair Automattic has that has different Jetpack functionality is Superhero/Sidekick, but that could change down the road. This could help make the message more accurate in those cases.

In plugin-enhancements.php - the enqueue_scripts and dismiss_admin_notice functions aren't prefixed, so users could run into already declared errors.

Both functions should be prefixed.

HT @kraftbj

This is an absolutely amazing tool for easily making the Custom Colors setting.

Is there any chance for any of these tools to be available at the NPM repository? I'd like to use it as a dependency in my theme development workflow/process. Currently, I place those getselectors and csspalette in a project subdirectory theme-tools/, reference it with package.json's "script" property and call it with npm run. I'd like to skip the first part to make it cleaner and easier to install/update.

Currently, the script mentions the word "commit to disk" which can cause confusion with an actual SVN commit. Change the wording to be less confusing.

Please note that this: https://github.com/Automattic/theme-tools/blob/master/jetpack-dependency-script/plugin-enhancements.php#L46-L51 will throw the following issue for WooCommerce welcome screen:

Notice: Trying to get property of non-object in /nfs/c03/h01/mnt/168730/domains/example.com/html/wp-content/themes/theme-name/inc/plugin-enhancements.php on line 55

I'm sure with others where the same is used.

The current user-facing wording in the notification box needs to be updated to better match the current Jetpack settings screens.

For example, the wording refers to modules, when Jetpack no longer refers to/organizes its settings in that way.

As another example, the wording talks about activating the Custom Content Types module for portfolio projects and testimonials, when testimonials and portfolios are now broken down individually:

Screenshot from Jetpack > Settings > Writing

A full audit should be done and all the necessary wording tweaked.

While it's possible to close the admin notification, it keeps coming back on page refresh, so it's not truly dismissible.
Reported here: https://themes.trac.wordpress.org/ticket/28319#comment:24

When running Sketch through the script, the tags don't get re-added to style.css header.

This can be tested by running through the script step-by-step to see where it's getting thrown off; there's likely just a formatting issue with the reges, where Sketch is using *s before each line in the style.css header.

I've tested a few other themes that use this style (there are fifteen in total, all older themes) and they all have the same issue. Either the themes should be adjusted to use a consistent style, or the regex used by the script should be adjusted to be more robust. (Will require testing!)

To avoid implying that Jetpack must be used with a theme, change the word "required" to "needed" throughout the script.

Recommended by WordPress.org reviewer at:
https://themes.trac.wordpress.org/ticket/30236#comment:10