Create / Inject JavaScript that turns configuration and url parameters into JS code.

Comments

joshmiller created an issue. See original summary.

joshmiller’s picture

joshmiller’s picture

Version: » 7.x-1.x-dev
Assigned: joshmiller » Unassigned
Status: Active » Needs review
StatusFileSize
new2.09 KB

Includes:

  • A preprocess function that injects JS if the path is correct.
  • A JS file that is ready to accept a JS function.
heddn’s picture

Status: Needs review » Needs work
  1. +++ b/customers_canvas.js
    @@ -0,0 +1,13 @@
    +(function () {
    ...
    +})();
    

    Pass Drupal and possibly jQuery into the js, Then alias them as Drupal and $ inside the script. It makes for easier debugging of JS later on. See https://www.drupal.org/docs/7/api/javascript-api/managing-javascript-in-... as an example.

  2. +++ b/customers_canvas.module
    @@ -139,3 +139,37 @@ function customers_canvas_builder($user_id, $entity_id, $entity_type) {
    +function customers_canvas_preprocess_page($variables) {
    

    The recommended way to add JS in D7 is via hook_page_build and add it as an #attached element. I think you can do the path checking in there instead. Or alternatively, attach it to the theme element rendered on the specific menu path.

  3. +++ b/customers_canvas.module
    @@ -139,3 +139,37 @@ function customers_canvas_builder($user_id, $entity_id, $entity_type) {
    +  // Only proceed if we're remotely sure the path is right.
    

    Why? what if someone wants to print things and they are using drush (which doesn't have any urls) or in a test path. This would essentially block that.

joshmiller’s picture

Status: Needs work » Needs review
StatusFileSize
new4.39 KB
new5.21 KB

Resolved all feedback, I think.

joshmiller’s picture

  1. +++ b/customers_canvas.api.php
    @@ -0,0 +1,38 @@
    + *   Keyed array that includes two variables that are used to include the
    

    Generify this comment, number of arguments is in flux for now.

  2. +++ b/customers_canvas.api.php
    @@ -0,0 +1,38 @@
    +    $current_node = menu_get_object("node");
    

    Whoops, unfinished thought there.

joshmiller’s picture

StatusFileSize
new4.57 KB
new5.39 KB
joshmiller’s picture

StatusFileSize
new992 bytes

Better interdiff

heddn’s picture

  1. +++ b/customers_canvas.api.php
    @@ -0,0 +1,44 @@
    +  if ($data['args']['owner_id'] == NULL) {
    

    This is weird. Can we check for empty or at least check for === for NULL?

  2. +++ b/customers_canvas.api.php
    @@ -0,0 +1,44 @@
    +    $current_node = menu_get_object("node");
    ...
    +    $data['args']['entity_type'] = "node";
    

    single quotes more typical.

  3. +++ b/customers_canvas.js
    @@ -0,0 +1,16 @@
    +(function ($) {
    

    Pass Drupal in as well. It would look like:

    (function ($, Drupal) {
        "use strict";
        // logic here.
    })(jQuery, Drupal);
    
  4. +++ b/customers_canvas.js
    @@ -0,0 +1,16 @@
    +      $('#cc_wrapper', context).once('customersCanvas', function () {
    
    +++ b/customers_canvas_builder.tpl.php
    @@ -10,5 +10,5 @@
    +<div class="customers_canvas" id="cc_wrapper">
    

    This tightly couples the theme layer to your js. And means the builder can only be printed a single time on a page. If that is acceptable, then proceed.

  • joshmiller committed 2a6fda7 on 7.x-1.x
    Issue #2951589 by joshmiller, heddn: Create / Inject JavaScript that...
joshmiller’s picture

Status: Needs review » Fixed

Thanks for the feedback, as always Lucas :)

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.