Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dependencies for tag visitors in Optimization Detective #1419

Open
westonruter opened this issue Jul 30, 2024 · 1 comment · May be fixed by #1469
Open

Dependencies for tag visitors in Optimization Detective #1419

westonruter opened this issue Jul 30, 2024 · 1 comment · May be fixed by #1469
Assignees
Labels
Needs Dev Anything that requires development (e.g. a pull request) [Plugin] Enhanced Responsive Images Issues for the Enhanced Responsive Images plugin (formerly Auto Sizes) [Plugin] Image Prioritizer Issues for the Image Prioritizer plugin (dependent on Optimization Detective) [Plugin] Optimization Detective Issues for the Optimization Detective plugin [Type] Enhancement A suggestion for improvement of an existing feature

Comments

@westonruter
Copy link
Member

In at least one case, there is a need currently for a tag visitor to run after another tag visitor has run:

/**
* Registers the tag visitor for image tags.
*
* @since 1.1.0
*
* @param OD_Tag_Visitor_Registry $registry Tag visitor registry.
*/
function auto_sizes_register_tag_visitors( OD_Tag_Visitor_Registry $registry ): void {
$registry->register( 'auto-sizes', 'auto_sizes_visit_tag' );
}
// Important: The Image Prioritizer's IMG tag visitor is registered at priority 10, so priority 100 ensures that the loading attribute has been correctly set by the time the Auto Sizes visitor runs.
add_action( 'od_register_tag_visitors', 'auto_sizes_register_tag_visitors', 100 );

The tag visitor in Enhanced Responsive Sizes needs to run after the IMG tag visitor be

The Image Prioritizer's tag visitor is currently registered with the name ID of img-tags which is not ideal:

// Note: The class is invocable (it has an __invoke() method).
$img_visitor = new Image_Prioritizer_Img_Tag_Visitor();
$registry->register( 'img-tags', $img_visitor );

It should rather be prefixed like image-prioritizer-img. The image-prioritizer-img string could even be defined as a class constant like Image_Prioritizer_Img_Tag_Visitor::ID. When Enhanced Responsive Sizes registers its tag visitor, it should be able to explicitly declare this dependency:

function auto_sizes_register_tag_visitors( OD_Tag_Visitor_Registry $registry ): void { 
	$dependencies = array();
	if ( class_exists( 'Image_Prioritizer_Img_Tag_Visitor' ) ) {
		$dependencies[] = Image_Prioritizer_Img_Tag_Visitor::ID;
	}
	$registry->register( 'auto-sizes', 'auto_sizes_visit_tag', $dependencies ); 
} 

Then there wouldn't be a need to rely on action priorities (and the current implementation of OD_Tag_Visitor_Registry) to ensure that one tag visitor runs before another.

@westonruter westonruter added [Type] Enhancement A suggestion for improvement of an existing feature [Plugin] Optimization Detective Issues for the Optimization Detective plugin [Plugin] Enhanced Responsive Images Issues for the Enhanced Responsive Images plugin (formerly Auto Sizes) [Plugin] Image Prioritizer Issues for the Image Prioritizer plugin (dependent on Optimization Detective) labels Jul 30, 2024
@westonruter westonruter changed the title Add dependencies for tag visitors in Optimization Detective Dependencies for tag visitors in Optimization Detective Jul 30, 2024
@westonruter westonruter added the Needs Dev Anything that requires development (e.g. a pull request) label Jul 31, 2024
@pbearne pbearne self-assigned this Aug 13, 2024
@westonruter
Copy link
Member Author

Note: The one current use case for tag visitor dependencies (Enhanced Responsive Images depending on Image Prioritizer) is going to be undone per #1476 (comment). The code the former is going to be moved to the latter.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Dev Anything that requires development (e.g. a pull request) [Plugin] Enhanced Responsive Images Issues for the Enhanced Responsive Images plugin (formerly Auto Sizes) [Plugin] Image Prioritizer Issues for the Image Prioritizer plugin (dependent on Optimization Detective) [Plugin] Optimization Detective Issues for the Optimization Detective plugin [Type] Enhancement A suggestion for improvement of an existing feature
Projects
Status: Not Started/Backlog 📆
Development

Successfully merging a pull request may close this issue.

2 participants