User Interface submodule

I am very interested in including a ui sub module.

I would like the complete git history if I could. Could you push the code to a sandbox project so I can pull that into the et (event tracking project).

if you wont have time to create the sandbox, you can still attach a patch.

I have commit this to another branch. I haven't tested the code yet though.

made my thinking clearer.

If you want to contribute to this branch please check out the code here
https://drupal.org/node/1342560/git-instructions/7.x-1.x-admin-ui

I'm investigating using the UI from comment 4 above as well. If all goes well, I'll go ahead and get the process started to get it included in a future version of this module.

Thanks,
-mike

uliimike, I think that is where the admin ui branch of the project comes from. I would want you to start there.

@frob,

I've gone ahead and created a patch for the 7.x-1.x-admin-ui branch that contains the following changes:

1. I've fixed the way the "Add selector" logic works to make it more robust.
2. I've improved the method for deleting selectors. Users can now delete multiple selectors at one time.
3. I fixed a small PHP warning having to do with non-Enabled selectors.

I should note that I rebased the 7.x-1.x-admin-ui on top of the current 7.x-1.x branch locally before starting, although I'm pretty sure the patch will apply without the rebase as well.

Patch attached.

Note that this patch won't apply to the 7.x-1.x branch and will therefore not pass the automated d.o. tests unless it runs against the 7.x-1.x-admin-ui branch...

Thanks,
-mike

I've updated the patch from comment 9 with a few additional changes:

I've incorporated most of patch 7 from #1778250: Allow Tokens from other DOM elements. This allows site admins to specify custom replacement tokens (no, not [those tokens](https://drupal.org/project/token)).
I've fixed the sanitization function for selectors.
I've added a text area to the UI to allow site admins to enter custom tokens.

I still can't set the version of this issue to 7.x-1.x-admin-ui (so that the automated tests will run), as it is not one of the options. That being said, a large organization is testing this patch and functionality and I'll update the patch as we progress, if necessary.

Thanks,
-mike

Wish I would have seen this before beginning my work :) I've created a UI submodule as well, with the difference mainly being that it is entity based rather than using the variables table. Our sites will be creating a great number of tracking arrays and we were concerned about performance in the variables table. I'm offering this as an alternative or we could work on combining the best of both submodules.

My only issue with it being entity based is that it then becomes content. A ctools exportable version would be better. I used the variable table because strongarm makes that easy to export.

A simpler solution would be to break this up across more variables. How many is a great number?

@frob For my part the reason I chose an entity approach is specifically because I do see these as content. My thinking is that they are directly related to other content on the site (links, images, etc) and so will be maintained by the site owners rather than implemented by site builders/developers. At least thats how we intend to use them; allowing site owners to create these as needed as they create content. Having them as an exportable for Features was also something we were specifically avoiding; for instance we wouldn't want to have them back ported down to our stage or dev sites as committed features and potentially confusing the event tracking information.

Seems like 2 very different approaches/implementations for this; is there a way to accomplish both?

@jhenson
Lets move this into another thread. I wouldn't want this hyjack the work that has already been done on the existing architecture.

As said in #2345665: Discuss the possibly of moving et from variables into entities.: CTools exportables are preferred over entities.

This patch is based on the 7.x-1.x-admin-ui branch and contains:

• CTools exportables for selectors.
• Backward compatibility to the old exportables defined in hooks.
• Changes of 7.x-1.x merged into 7.x-1.x-admin-ui
• Code refactoring
• Updated google_analytics_et.api.php with the new hook definitions.

Still to do:

• Provide an update hook:
  • To create the table if someone was using the module from this branch
  • To update old settings defined in variables to the new ctools export objects
  • To delete the old variables which are no longer used
• Update the example module. I didn't do this on purpose for now so it's clear that it works with the old hook definitions as well.

I ran out of time to work on this, so anyone can feel free to take over and write the updates.

I tested this patch with the following steps:

• Enable the example module to see the old hook definitions still work. They should show up in the UI.
• Override the defaults from the example module and see if they stick. They should.
• Create a new setting in the UI.
• Test the settings by seeing if the Google Analytics calls are made when and how they should.

Maybe I should have added the patch... ;-)

@Jelle_S, I am sorry I need to clean up this repo a bit. Can you please reroll this patch based on dev? admin-ui was only used to ease the testing and I am not sure out of sync it is with dev at this point.

Rerolled branch 7.x-1.x-admin-ui and most recent patch against the latest dev version.

Fixed coding standards issues.

i'm having some issues trying to get this to work. either i don't know what i am doing (entirely possible), or something is broken. upon applying the patch, i get this error:

Fatal error: Call to undefined function ctools_include() in .../sites/all/modules/google_analytics_et/google_analytics_et.module on line 64

the patch wouldn't apply to the UI branch, so i attempted the 7.x-1.x-dev branch and it patched but gave the white screen error

i really would like to get this module working on a site i have! thanks for all the work. let me know what i can do to help.

EDIT: i downloaded and enabled ctools. we should add it to the dependencies. still working on getting this to work...

@BiigNiick
will you please send me the drupal core and the.module versions plus the full error messages.

I have this patch applied and work on a production site with no issues.

Rerolled the patch to fix some coding standard issues.

@BiigNiick I think that you didn't enable the new submodule google_analytics_et_ui which has ctools as a dependency. That's why you got the ctools code error.

The objective of this patch is to add google_analytics_et_ui submodule.

Switching issue status into Needs review since the patch test results failed for the dependency google_analytics and passed for this patch.

check http://www.drupal.org/pift-ci-job/842330 for more details.

@mqanneh, i'm finding it may have been that a client's purchased a theme that was (among other things) overriding jQuery and making js things break. i think it's working as designed, but i don't have it up and working properly yet... thanks for the response.

I have set a retest on php 5.6, I am thinking it might be a php version issue over on the GA side. Using [] syntax instead of array()

Looks like the GA dev module is fixed. http://cgit.drupalcode.org/google_analytics/commit/googleanalytics.test?...

If I can get one RTBC on this patch then I will commit it and at least this will be in dev. I don't use this module any more as I am no longer in an industry that benefits from the type of broad AB testing that this module provides, thus I do not have a site to test this on.

@fob I'm using this patch on a production site with no issues. Looks like the automated test result errors are related to php version as you mentioned,

The error is raised from google_analytics module and it's not related to this patch.

PHP Parse error: syntax error, unexpected '[' in /var/www/html/sites/all/modules/google_analytics/googleanalytics.test on line 512

Okay, so I spoke too soon. I just reviewed the patch. Why does it change id to name. As far as I can tell this patch shouldn't edit the js at all.

While at DrupalCon Nashville I got to review this patch much more closely. In order for this patch to be accepted it must make no changes to the existing js and module file. Not that the changes are not good, it makes too many of them and most of them are out of the scope for this patch.

The reason for this is to maintain backward compatibility with the existing api. This means also maintaining backward compatibility with dependencies. This patch would make ctools a dependency of both the ui and the api.