Updates from Jeremy Felt Toggle Comment Threads | Keyboard Shortcuts

  • Jeremy Felt 10:22 pm on August 24, 2012 Permalink | Reply  

    Trying to make it easier to understand how ACM could apply to various DFP (JS API) setups. I think I’ve wrapped my mind around a few ways to target things via DFP and I’m building up some sample scripts for each scenario. Is anything in this list crazy?

    1) The ad unit in the header template will always be ‘header_728x90’ (*sitewide*) and all targeting will be done via orders created in DFP based on unit/time/etc.

    2) The ad unit in the header template is more dynamic and could be called as ‘header_728x90’ or ‘header_alt_728x90’ or …, it is targeted via orders in DFP but needs to change depending on the view – home, category, single, etc…

    3) The ad unit in the header will always be ‘header_728x90’ (*sitewide*), but key/value pairs will be targeted via orders in DFP that should be included with the ad call depending on the page view – home, category, single, etc….

    4) Combo of 2 & 3

    I’ve started a gist here – https://gist.github.com/3441995 – that I’m building out some more.

    One of the things that’s come up during this exploration is wondering how useful ACM is to somebody that is using scenario 1 above. Also, scenario 4 just scares me… 🙂

     
    • Jeremy Felt 10:23 pm on August 24, 2012 Permalink | Reply

      Also, we need Markdown for P2 on here… 😉

    • Rinat K 11:27 pm on August 24, 2012 Permalink | Reply

      When I did setup for DFP JS I just created the necessary slots and named them arbitary and matched slots with ad tag ids.

      One problem that I faced with DFP JS was .enableSingleRequest()
      If you defineSlot() 5 ad tags, but try to render only three of them, none will render properly. So I ended up commenting it out.

  • Jeremy Felt 12:05 am on May 22, 2012 Permalink | Reply  

    I ran into an issue with all columns being required when creating/editing an ad code – notes are up on GitHub.

    My current thought is to solve this by adding a filter to action_load_providers() that sets something like $optional_provider_columns that we can check alongside $ad_code[$slug] in create_ad_code() and edit_ad_code().

    Does that sound reasonable? I can’t find anything that would break as a result of this change, but I’m still hunting.

     
    • Daniel Bachhuber 4:51 pm on May 23, 2012 Permalink | Reply

      To be honest, I’m not entirely sure of the origin of that code. It seems like the best solution might be to keep the fields required for an ad code independent of the columns that appear, but then have them influence which columns appear. Something like this:

      $this->ad_code_args = array(
      	array(
      		'key'       => 'site_name',
      		'label'     => __( 'Site Name', 'ad-code-manager' ),
      		'editable'  => true,
      	),
      	array(
      		'key'       => 'zone1',
      		'label'     => __( 'zone1', 'ad-code-manager' ),
      		'editable'  => true,
      	),
      );
      $this->columns = array();
      foreach( $this->ad_code_args as $ad_code_arg ) {
      	if ( !$ad_code_arg['editable'] )
      		continue;
      	$column = array(
      		$ad_code_arg['key'] => $ad_code_arg['label'],
      	);
      	$this->columns = array_merge( $this->columns, $column );
      }
      $this->columns = apply_filters( 'acm_provider_columns', $this->columns );
      

      I think we’d want to standardize it a bit more and stick it in the base class so it auto-populated $this->columns based on the args. Or, drop $this->columns altogether and build our columns based on $this->ad_code_args

      • Jeremy Felt 5:58 pm on May 23, 2012 Permalink | Reply

        If I’m reading it right, the structure is on the right track, but we need another argument for ‘required’ when defining the codes. Rereading my GitHub comment, I’m not even sure I conveyed the problem correctly. 🙂

        $this->ad_code_args = array(
            array(
                'key'       => 'site_name',
                'label'     => '__( 'Site Name', 'ad-code-manager' ),
                'editable'  => true,
                'required'  => true,
            ),
            array(
                'key'       => 'zone1',
                'label'     => __( 'zone1', 'ad-code-manager' ),
                'editable'  => true,
                'required'  => false,
            ),
        );
        

        In the case above, both site_name and zone1 would be editable, but the user could leave zone1 blank when saving the code.

        The second part of the problem occurs in create_ad_code() and edit_ad_code(), where we would need to check that required flag.

        function create_ad_code( $ad_code = array() ) {
            $titles = array();
            foreach ( $this->current_provider->columns as $slug => $col_title ) {
                // We shouldn't create an ad code,
                // If any of required fields is not set
                if ( ! $ad_code[$slug] ) {
                    return;
                }
                $titles[] = $ad_code[$slug];
            }
            //....
        }
        

        An additional check could be added to if( ! $ad_code[$slug] ) or we could take that out entirely and find a way to catch the field as a required input before the form was submitted.

        • Daniel Bachhuber 6:15 pm on May 23, 2012 Permalink

          You’re right, I missed the ‘required’ argument. The create_ and edit_ functions would obviously need to be updated to properly validate the ad codes. Eventually we could add a ‘type’ argument to ad codes and/or a whitelisted set of values to offer better form validation.

        • Rinat K 9:29 pm on May 23, 2012 Permalink

          Sorry guys, I kind of got out of the loop on this one. I like $this->ad_code_args solution!

        • Rinat K 4:15 am on May 25, 2012 Permalink

          Hey,

          I did some work on known issues, so this is only one outstanding for 0.2.2 release. Well, there’s another one, but it’s related to it.

        • Rinat K 6:26 am on June 7, 2012 Permalink

          Hey @danielbachhuber @jeremyfelt, i made a swing, it is in a branch refactor-providers.
          I didn’t test thoroughly but my fairly rudimentar test suite (in a branch tests) shows that at least creating and getting ad codes working as expected.

c
Compose new post
j
Next post/Next comment
k
Previous post/Previous comment
r
Reply
e
Edit
o
Show/Hide comments
t
Go to top
l
Go to login
h
Show/Hide help
shift + esc
Cancel