deshack / pinit

Handy plugin that adds Pinterest Follow Button, Pin Widget, Profile Widget and Board Widget to your WordPress site.
GNU General Public License v2.0
4 stars 2 forks source link

Use wp_add_inline_script() in WP 4.5 #18

Closed deshack closed 7 years ago

deshack commented 8 years ago

WordPress 4.5 (Beta 1 to be precise) introduces wp_add_inline_script() for including inline JavaScript just like wp_add_inline_style() works for CSS. See the release notes.

wp_add_inline_script() takes dependencies into account (not really useful in our case though) and also adds the ability to filter the code, so that something like what suggested to capabomba would be accomplished by filtering the inline script and not removing the action and adding a new one.

EugenioPetulla commented 8 years ago

Ok, i will implement this filter in the next release within a requirements check to ensure the backward compatibility. :)

deshack commented 8 years ago

@EugenioPetulla it's ok for you to bring this as soon as in v2.0.1?

EugenioPetulla commented 8 years ago

I think it's a little bit too soon, because WP 4.5 will be out only in April and I'm planning to rollout some little updates and fixes so Maybe it's better to put this in 2.2.0 :)

EugenioPetulla commented 8 years ago

I wait until WP 4.5 will be released before setting a milestone for this task.

deshack commented 8 years ago

I would not wait for the stable 4.5 release to come. It's enough to wait for the RC, so that when 4.5 is released, we're ready with it.

What do you think @EugenioPetulla ?

EugenioPetulla commented 8 years ago

Good to me. RC and then we will set the milestone. :+1:

EugenioPetulla commented 8 years ago

I checked progress of the wp_add_inline_script() but it's not useful to us because it's not suitable for include external scripts and manipulate the script tag in order to inject some html meta-data we need for the trick.

Actually what we have to embed is:

`

            $yoda = ' data-pin-hover="true"';
                            if( !empty( $this->settings[ 'round' ] )) {
                $yoda .= ' data-pin-round="true"';
            }

            else {

                if( isset( $this->settings[ 'color' ] )){
                    $yoda .= ' data-pin-color="'. $this->settings[ 'color' ] . '"';
                }

                if( isset( $this->settings[ 'language' ] )){
                    $yoda .= ' data-pin-lang="'. $this->settings[ 'language' ] . '"';
                }
            }

            if( !empty( $this->settings[ 'large' ] )) {
                $yoda .= ' data-pin-tall="true"';
            }

        echo '<script async defer' . $yoda . ' type="text/javascript" async src="//assets.pinterest.com/js/pinit.js"></script>' . "\n";`
deshack commented 8 years ago

@EugenioPetulla I don't really see the point here. I don't really know how wp_add_inline_script() works, so it probably does not properly support HTML meta-data natively, but..is it really not worth?

Also, why isn't it suitable for external scripts?

EugenioPetulla commented 8 years ago

Because it is just for inline script (like the function name suggests) and it's able to append just <script></script> or <style></style> tags and there are no parameters to set attributes like we need.

I give you an e.g. of the new function usage and what is made for:

add_inline_script( 'jquery', '!window.jQuery && otherJS('here')', false );

This will output the inline script in the footer.

As you can see there are no parameters to append an html data-attribute to the script tag, so it's impossible to manupulate the output like we are doing printing it directly in the footer. Plus, we have no dependences to manage, no conflict modes to think about or other exotical issues so, to me, we are right on the track at moment.

We can consider to put a custom filter in the function so we can provide an external filtering on the script made for coders and for fast issue recovering. But we have to open another issue if we want go for it! :)