How this plugin handles paths

[Levdbas]

Related:

• How this plugin handles paths. #22

There are still some todos before we can review this:

• make the way the __construct() method is setup more flexible.
• test icons in parent theme
• test themes without a parent theme
• test custom location override
• set a path that is a sane default in acf_icon_path_suffix or the new filter that we will use instead.
• Deprecate old filters
• Rewrite readme file

Issue

The way you set paths, url's and directories via filters is not very handy.

Solution

Rewrite the way you can serve SVGs to this plugin.

Impact

High, users will have to use the new filters to keep using this new version. There will be a fallback for the most simple setups that will throw a deprecation notice.

Usage Changes

Some, but I will document that later.

Considerations

Might be a step too far, although I think this is way more flexible.

Testing

Not yet, something to consider later

[Levdbas]

Hi @mike-sheppard ,

PR is ready for review. I still have to document the changes. I also set up PHPunit tests that I do need to document as well in a contribution guide. If you want to lend a hand in this as well, feel free to do so.

New default theme location
We can discuss this, but I think this is a sane default: themes/my-theme/icons/

There are two new filters

For setting a custom folder inside your (parent)theme:

// will look into themes/my-theme/custom/icons/ for icons and also in the parent theme in the case of parent/child themes.
add_filter('acf_svg_icon_picker_folder', function () {
	return 'custom-icons/';
});

and for setting a complete custom location:

// takes a path and url location
add_filter('acf_svg_icon_picker_custom_location', function () {
	return [
		'path' => WP_CONTENT_DIR . '/random-location-icons/',
		'url' =>  content_url() . '/random-location-icons/',
	];
});

There are three new helper functions:
These can help plugin users to get the correct icons in parent/child theme setups. They wont currently work with custom location. But then again, if you are using a custom location, you can write your own getter function anyway.

SmithfieldStudio\AcfSvgIconPicker\get_svg_icon_uri(),
SmithfieldStudio\AcfSvgIconPicker\get_svg_icon_path() and
SmithfieldStudio\AcfSvgIconPicker\get_svg_icon()

[mike-sheppard]

ah nice one @Levdbas all sounds great to me, will pull this down and give it a test later this week. Thanks again for the hard work on all of this 🙏

[mike-sheppard]

hey @Levdbas tested this out, and it works perfectly. While digging around, I managed to fix an issue we have with hashed assets #26 - hoping this simplifies things a tiny bit 🤞

[Levdbas]

We have automatic tests now @mike-sheppard 👯

[Levdbas]

@mike-sheppard , can you do one last check on my readme changes and contribution guide? I documented there how to run the tests locally.

[mike-sheppard]

Nice one! Left a few minor comments + still need to get my tests running locally

[Levdbas]

@mike-sheppard , shall I invite you to a quick Teams call to get the tests running? Or else we can schedule something for next week.

[mike-sheppard]

All tests passing 🎉 I'll quickly add a test to my branch for the extra function added, test + merge back here for you to release when ready 🙏

[mike-sheppard]

Reckon this is good to go whenever you're happy to merge! 🙌

[Levdbas]

Merged! @mike-sheppard , can you do a changelog update as well before deploying the update? Probably emphasize a bit on the deprecated filters and the new way of saving the file names. Enjoy the weekend and amazing work!