Updates from February, 2013 Toggle Comment Threads | Keyboard Shortcuts

  • Daniel Bachhuber 9:12 pm on February 28, 2013 Permalink | Reply
    Tags: ,   

    … and I’m back into it. Let’s discuss a launch date for v0.4. I think the easy Google AdSense / Google DFP configuration is a good highlight feature.

    @rinatkhaziev @jeremyfelt Of the other features we lined up for the milestone, is there anything you’d like to try to finish up before release?

    I think two or three weeks from now would be a good date to hit.

  • Rinat K 4:55 am on June 27, 2012 Permalink | Reply

    0.3 Ideas Brainstorming Post 

    The 0.2 branch is looking good, so let’s talk 0.3. I recalled some ideas that were floating around since 0.1, but were never actualized:

    • Configuration scanner: verify configuration checklist ( provider file is present, ACM_WP_List_Table columns are properly defined, ad code args are set correctly, there’s at least one tag id, etc)
    • Ad Code Preview: This should be probably a row action which either pops an overlay with previews or displays them inline
    • Configuration builder: this one might be an overkill( aka punt candidate or fuggedaboutit), but it would be nice to have some tool that would generate configuration filters. Basic worklow would look like: user chooses one of existing ad providers, and modify some of default settings. For an average user, who downloaded plugin and who doesn’t know much about PHP coding, an ability to visually configure Β  stuff would come in handy.

    What do you think on these points? Do you have any other ideas/solutions? I personally appreciate the idea of keeping things contained and not over-bloated, but users get confused over configuration. I mean, one have to define 5-10 filters to get things up and running. Β This could be a deal-breaker for some folks.

    • Jeremy Felt 6:39 pm on June 27, 2012 Permalink | Reply

      I like the idea behind a configuration scanner / preview. I think it may be useful to combine those two and possibly some other data in some way as part of an add on for Debug Bar that is included with ACM. This may make it a lot easier to debug ad issues without having to dig through source, etc.

      The configuration builder does seem a bit overkill, but I do think we need an easier way to explain ACM to a new developer. I think I’ve tried 3 different approaches in our team now and it’s taken several conversations to get minds wrapped around how it works. The big question is how to quickly apply ACM to a non standard DFP configuration.

      From an end user perspective, it would be cool if we could modify the input fields for each conditional either through filters or just by default. For example – If is_category() is selected as the conditional, a drop down list of existing categories would be more pleasant than a text field.

    • Jeremy Felt 10:31 pm on June 27, 2012 Permalink | Reply

      Another idea that just came up – allowing for multiple providers to coexist on a page.

      It seems doable now if you know provider 1 will always display in the header and provider 2 will always display in the sidebar, but I can’t find a way to hook in via filter and change that based on an ad_code_arg to allow for complete flexibility for the user.

      • Daniel Bachhuber 10:37 pm on June 27, 2012 Permalink | Reply

        allowing for multiple providers to coexist on a page.

        This seems like an awesome improvement. File an issue πŸ™‚

    • Daniel Bachhuber 10:38 pm on June 27, 2012 Permalink | Reply

      I think a configuration scanner would be a great feature for v0.3. We could probably stick to one large feature per release to keep our release cycles reasonable (do what I say not what I do).

      • Rinat K 9:30 am on June 28, 2012 Permalink | Reply

        I went ahead and filed all the ideas. Feel free to tackle them πŸ˜€

    • David 9:01 am on September 18, 2012 Permalink | Reply

      Can I chime in here as a new to DFP (SB) and new to ACM user? It’s all confusing as hell. πŸ™‚

      Documentation, hand-holding and setup wizards would (I think) go a long way for better adoption. I’m trying to muddle my way through things with limited success. For example: what’s zone1? Never explained. It’s probably super-obvious to an experienced user but it’s never explicitly defined anywhere.

      All that said, this was/is obviously a lot of work so, thank you for creating it. If I can get this going I know it’ll make my life easier in the future.

      Where is the best place to post questions? Here, on the WP forum page?

  • Daniel Bachhuber 11:56 pm on June 25, 2012 Permalink | Reply

    Synced over the extent of this commit to WordPress.com VIP (except the unit tests).

    If you could commit against an issue when you’re making improvements, it would be much appreciated. It’s much easier to refer to a series of commits this way πŸ™‚

    Thanks guys

    • Rinat K 4:25 am on June 26, 2012 Permalink | Reply

      @danielbachhuber I closed the outstanding issue and tagged 0.2.3 on github, wanted to push to WPORG but found out that I have no write privileges. So either give me a write or tag it yourself, please πŸ™‚

      • Daniel Bachhuber 7:30 pm on June 26, 2012 Permalink | Reply

        Sorry about that, I’ve given both you and @jeremyfelt commit access.

        When you tag the version, can you also remove the .js and .css files from jqGrid that are no longer in use? I’ve been unzipping the new version into the directory, which means changed files get updated and new files get added, but old files aren’t deleted…

        I’ve already removed the old files from the VIP repo.

        • Rinat K 8:44 pm on June 26, 2012 Permalink

          I found out that rsync comes in pretty handy in those situations

        • Rinat K 4:09 am on June 27, 2012 Permalink

          on the other hand, rm & svn rm are ok too πŸ™‚

        • Mo Jangda 4:11 am on June 27, 2012 Permalink

          rsync -rv --delete --exclude='.svn' /from /to
          cd /to
          svn status | grep '^!' | awk '{print $2}' | xargs svn rm
          svn status | grep '^?' | awk '{print $2}' | xargs svn add


        • Rinat K 4:14 am on June 27, 2012 Permalink

          Ha, glad that you’re still with us and thanks for the snippet πŸ™‚

        • Jeremy Felt 3:06 am on June 28, 2012 Permalink

          It should be known that Mo’s rsync comment made way for a beautiful addition to my local bash profile

  • Rinat K 7:21 pm on June 7, 2012 Permalink | Reply  

    Hey guys, there’s one issue left: https://github.com/Automattic/Ad-Code-Manager/issues/26

    What are your thoughts on it, @danielbachhuber, @jeremyfelt

  • 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(
      		'key'       => 'site_name',
      		'label'     => __( 'Site Name', 'ad-code-manager' ),
      		'editable'  => true,
      		'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'] )
      	$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(
                'key'       => 'site_name',
                'label'     => '__( 'Site Name', 'ad-code-manager' ),
                'editable'  => true,
                'required'  => true,
                '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] ) {
                $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


          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.

  • Daniel Bachhuber 2:53 pm on May 4, 2012 Permalink | Reply

    @rinatkhaziev @jeremyfelt how are you guys feeling about v0.2? Release today or Monday?

  • Daniel Bachhuber 6:00 pm on April 30, 2012 Permalink | Reply

    Finished refactoring the WP List Table implementation this morning. Here’s what I’ve identified as needing work for v0.2:

    @jeremyfelt want to take a swing at updating the readme and including screenshots? @rinatkhaziev have time to start testing? I’m cool with releasing this as soon as I get the checkoffs from you guys πŸ™‚

  • Rinat K 2:10 am on April 19, 2012 Permalink | Reply
    Tags: ,   

    Had some time today to remove jqGrid related code and add contextual help. Also added zztimur (our UX specialist) and jeremyfelt as contributors.

    There is a couple of things we need to do before tagging it 0.2:
    add/delete conditionals in “edit” row action
    delete action for ad codes

    • Jeremy Felt 6:33 am on April 19, 2012 Permalink | Reply

      @rinatkhaziev I can commit time to wrapping up the add/delete conditionals in the edit row and the delete action for ad codes at some point tomorrow. May be later in the day, but definitely doable.

    • Rinat K 5:07 am on April 20, 2012 Permalink | Reply

      @jeremyfelt well this is last thing that holding us off from tagging it v0.2. Good thing is that there should be only js related functionality on add/delete conditionals. I probably will have some time to implement mass delete action and factoring out 0.1.3 request handling tomorrow. Thanks a lot for your help!

      • Jeremy Felt 6:28 am on April 20, 2012 Permalink | Reply

        @rinatkhaziev – I just pushed the code for adding conditionals and allowing conditionals to be removed completely.

        That’s it for that pull request, so you’re clear if everything looks good. I’ll read through the documentation over the next few days as well.

    • Daniel Bachhuber 8:46 pm on April 20, 2012 Permalink | Reply

      Implemented some functionality to see the conditionals as the last column in the list table.

      There’s a bit to be desired with this implementation of WP List Table, so I’m going to commit a few improvements against this ticket. I also highly encourage you guys to open tickets for the things you’re working on… this helps tremendously in figuring out what changed when it comes time to testing and release notes.

      @jeremyfelt you’re now a committer to the repo. congrats πŸ™‚

  • Rinat K 5:41 pm on April 11, 2012 Permalink | Reply
    Tags: ,   

    Last night I committed more UI changes, however, still running late and won’t have any time today, probably. Any help with UI would be much appreciated.

    Here’s what’s left:
    Properly implement row actions
    Hook up AJAX editing actions for ad codes and conditionals
    When user adds ad code in the beginning, he could also add conditionals in one step. There’s a “Add more conditionals”, it should duplicate conditional.
    Minor css issues

Compose new post
Next post/Next comment
Previous post/Previous comment
Show/Hide comments
Go to top
Go to login
Show/Hide help
shift + esc