AlchemyUnited
commented
2 years ago Oh. I wanted to add, this is the constructor for UtmDotCodes at this point
public function __construct() {
require_once 'shorten/interface.php';
remove_post_type_support( self::POST_TYPE, 'revisions' );
}I would imagine those can be placed elsewhere, esp the require_once. The class shouldn't have a dependency like that.
Description
I was recently tasked to add the shortener service Cuttly to your plugin :) btw, I did see the bit in the wiki for using the interface provided, etc. (Thanks! That was nice and it was helpful). But I wanted to continue to use the UI to enter the API key**. Why not?
Well...
Since the hooks are within a class, and an instance of that class wasn't available. I wasn't able to use remove_action() for the callback that rendered the UI and then replace that UI with my own. That is, remove_action() needs the instance of the class as well as the callback in order to do the remove. (Actually, I did use the global for $wp_hooks and mucked through that but that's dirty / hacky. I don't like doing that.)
I've seen the plugin's (anti) pattern before and decided (1) I'd do a pull request (2) To write a blog post on that refactoring and why. Why? Classes should be what. The hooks are when. They should not be mixed together.
Finally, you'll recognize - and hopefully appreciate - I took it a step further. Not only are instances of 2 of the class available via filter, but the hooks are now setup via arrays and those arrays can also be manipulated via filters. Now the plugin is much more dev-friendly, much easier to extend.
That is, instead of adding and then removing, the hooks can be pro-actively manipulated instead of add / remove.
It's a bit heavy on the eye seeing those arrays but in terms of flexibility, A LOT is gained in doing it this way.
imho, of course ;)
** I don't think you should recommend hardcoding the API key in any class since that might get committed to a repo, etc. Mind you, most people should know better, but that isn't always the case. Sure, they also add their own admin settings UI but that's extra work for an UI that's already there and able.
p.s. I also did a bit of clean-up in Class UtmDotCodes_Activation. Similar to above. An array makes it cleaner and make it easier "mirror" Add option and remove option. They both work off the same array now.
Fixes # (issue)
Type of change
Please describe the type of change.
How Has This Been Tested?
I did not run any tests. Sorry.
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration
Test Configuration:
whatever the current up to date defaults are in Local (aka Local by Flywheel).
Checklist: