INN / doubleclick-for-wp

WordPress plugin for serving Google Ad Manager ads
https://wordpress.org/plugins/doubleclick-for-wp/
GNU General Public License v2.0
25 stars 11 forks source link

sizeMapping issue #22

Closed trevsewell closed 7 years ago

trevsewell commented 7 years ago

I'm using this plugin, working great.

however, when using the sizeMapping for multiple breakpoints. I had to cast minwidth value as an integer (I see this has been updated already). however you are providing the browserheight as the second parameter as 1.

it should infact be:

(int)$breakpoint->maxWidth;

please correct me if i'm wrong

benlk commented 7 years ago

Just to make sure I understand what you're talking about, you're asking about the sizeMapping portion of the code that is placed on the page?:

jQuery('.dfw-unit:not(.dfw-lazy-load)').dfp({
    dfpID: '65246486',
    collapseEmptyDivs: false,
    setTargeting: {"Page":["admin-bar-showing","single"],"Category":["arts-culture","entertainment-nightlife"],"Tag":["anthony-the-poet","center-city-development-and-operations-department","literary-arts","mission-marque-plaza","rebecca-viagran"]},
    sizeMapping: {
        "mapping1":[
            {"browser":[1220,1],"ad_sizes":[[728,90]]},
            {"browser":[1040,1],"ad_sizes":[[728,90]]},
            {"browser":[720,1],"ad_sizes":[[320,50]]},
            {"browser":[0,1],"ad_sizes":[[320,50]]}
        ],
        "mapping2":[
            {"browser":[1220,1],"ad_sizes":[[728,90]]},
            {"browser":[1040,1],"ad_sizes":[[728,90]]},
            {"browser":[720,1],"ad_sizes":[[320,50]]},
            {"browser":[0,1],"ad_sizes":[[320,50]]}
        ]
    }
});
benlk commented 7 years ago

The JavaScript in my previous comment is taken from the DOM; it's generated as a result of DoubleClick::enqueue_scripts:

        $mappings = array();
        foreach ($this->adSlots as $ad) {
            if($ad->hasMapping()) {
                $mappings["mapping{$ad->id}"] = $ad->mapping();
            }

        $data = array(
            'networkCode' => $this->networkCode,
            'mappings' => $mappings,
            'targeting' => $this->targeting()
        );
        wp_localize_script( 'jquery.dfw.js', 'dfw', $data );

$ad->mapping() is DoubleClickAdSlot::mapping, which sets the width and heights seen in the sizeMapping:

            // The minimum browser width/height for this sizemapping.
            $browserHeight = 1;
            $browserWidth = (int) $breakpoint->minWidth;

            $sizeStrings = explode(",",$size);  // eg. 300x250,336x300
            $sizeArray = array();

            // trimmed some irrelevant-to-this-ticket code here...

            $mapping[] = array(
                'browser' => array($browserWidth,$browserHeight),
                'ad_sizes' => $sizeArray
            );

The 1s that you're asking about aren't the max width. They're the minimum height of the browser for the ad to display. It's a DoubleClick feature that, for example, would allow you to set 600px-tall ads to only display if the window was taller than 612px. Here's some more information about how it works in DfP: https://support.google.com/dfp_premium/answer/3423562?hl=en

It's also described in the Available Options section of the jQuery.dfp.js docs.

The array is of minimum width and minimum height, so, no, it should't be (int)$breakpoint->maxWidth;

Please let us know if you have any other questions!