clarity icon indicating copy to clipboard operation
clarity copied to clipboard

Ability to add own selectors without the need to re-build the definitions.php

Open jmslbam opened this issue 2 years ago • 5 comments

Hi there,

Are you open for a PR that allows to add the ability to add your own definitions without the need to re-build the definitions.php.

Because currently wp_clarity_rules - which I'm using -, only runs after you built the plugin's definitions.php. And that logic just doesn't hold up because it gets overwritten with an plugin update.

And I don't want to run the command to generate the definitions after each deployment (manually or by hand). Also, sometimes I can't run a wp-cli command.

It could be as simple as making $loadFromSource filterable on the line https://github.com/khromov/clarity/blob/master/clarity-ad-blocker.php#L37. Otherwise it would need a bit more work / rewrite.

Ciao,

Jaime

jmslbam avatar Aug 12 '22 11:08 jmslbam

Hey @jmslbam ,

Yes, I'm open to it! It's an oversight that the filter only runs when using the build command. I don't actually think we need a filter for the build step, instead the wp_clarity_rules filter should be merged with the existing rules from definitions.php, something like:

add_filter('wp_clarity_rules', function($rules) => {
    return array_merge($rules, ['.big-ad', '#another-big-ad']);
});

At some point I also want to add a simple admin page where people can add extra rules through wp-admin and this filter would be great to use there as well.

khromov avatar Aug 12 '22 12:08 khromov

Hi,

It does mean that we need change thedefinitions.php to return an array. This always removes the need to build it and implode the array on-the-file.

  /**
   * Generate definitions from definitions.txt
   *
   * @return string
   */
  function getDefinitions(): array {
    $definitions = include_once WP_CLARITY_PATH . 'definitions.php';  
    
    $definitions = apply_filters('wp_clarity_definitions', $definitions );

    return array_filter( $definitions );
  }

  function getSelectors(): string {
    return implode(', ', $this->getDefinitions() );
  }

  /**
   *  Hides stuff via CSS in the admin header
   * 
   * @return void
   */
  function admin_head() {
    $selectors = $this->getSelectors();
    if (strlen($selectors) === 0) {
      return;
    }
}
// definitions.php 
<?php

return [
    // Yoast
    '.wpseo_content_wrapper #sidebar-container',
    '.yoast_premium_upsell',
    '#wpseo-local-seo-upsell',
    // WP Mail SMTP
    '#wp-mail-smtp-notice-bar'
];

I do understand the need to "cache" the selectors, but I think that mechanism can be solved when we really notice any performance issues. We then could cache the file into wp-content/cache/clarity/selectors.php

Otherwise we can keep definitions.php as a string and I need to apply the filter in getSelectors() which gets the string, and implodes the filters and concatinates that.

Lemme know which way you prefer. Mine would be simplicity to skip the build step in general and use implode.

jmslbam avatar Aug 12 '22 16:08 jmslbam

@jmslbam Thanks for the feedback! The original idea for definitions.txt was to provide a non-executable file that in the future could be background-updated by Clarity from a master copy on GitHub Pages to avoid having to update the plugin for each rule change. The downloaded file has to be non-executable because of the WPorg plugin guidelines. I think this is the best approach long-term.

What we could do for now is to make getDefinitions() return an array of rules like you proposed, so the output of definitions.php after building would be something like:

<?php
/* This file is automatically generated, do not update manually! Use 'wp clarity-build' to generate. */ 
return ['#rule-1', '#rule-2'];

This would make the proposed filter easy to implement and also avoid the biggest performance annoyance which is to parse the text file. And in the future it will be easy to refactor the definitions to be updated & built in the background and stored in wp_options without changing the wp_clarity_rules filter signature.

Let me know what you think.

khromov avatar Aug 13 '22 00:08 khromov

Also very open to a 'filterable' solution here 😃

E-VANCE avatar Sep 23 '22 07:09 E-VANCE

Added a PR #31 for this. I don't think there will be considerable performance issues with this approach. I'd recommend using transients rather than wp_options in case we need to cache.

mustafauysal avatar Nov 22 '22 21:11 mustafauysal