Since I don't have time to try and ajax enable the token tree, I'm going to try and alternate solution and encourage the token tree not to be output directly in page output, but rather encourage linking to 'View available tokens' which would open up a jquery UI dialog with the token tree inside of it.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Dave Reid’s picture

Oh my gosh, I think I have this working. Will post patch shortly.

Screenshot:
http://i.imgur.com/FIYgR.png

Dave Reid’s picture

Status: Active » Needs review
FileSize
4.25 KB
Dave Reid’s picture

Revised patch that fixes the CSS inside the modal only.

DamienMcKenna’s picture

Status: Needs review » Needs work

Is the patch missing something? I don't see what generates the link? With the current -dev codebase of both Token and Metatag, and this patch applied, I'm still getting the Ye Olde table underneath the Metatag fields.

Dave Reid’s picture

Status: Needs work » Needs review

We would have to change the implementing modules.

From:
'#theme' => 'token_tree'

To
'#theme' => 'token_tree_link'

That change is out of scope for this patch.

DamienMcKenna’s picture

Status: Needs review » Needs work

For anyone else who wants to test it, you can e.g. change metatag.module line 629 per Dave's comment in #5, or any other module that uses 'token_tree' to display a list of tokens.

Ok, this works pretty well.

There's a UX glitch at play that either is a core bug or something unique to this (I'm not sure off-hand). To reproduce:

  • Open the dialog.
  • Drag-n-drop the dialog to move it up the page. You might also resize it so it's smaller.
  • Scroll down.
  • Scroll down so that the dialog doesn't show on-screen at all, the detached table header will still remain on-screen.

Related to the above, should the table header float if the dialog is scrolled through? Right now it isn't, only when the *page* is scrolled.

I also wonder might it be better to just replace theme_token_tree entirely (hook_theme_registry_alter() if need be), rather than require modules update to using a new dialog? Maybe provide a variable to control which one is used, so that by default when the Token module is enabled you get the new dialog but can disable it if you need?

Dave Reid’s picture

I'm not sold on changing the default behavior of theme_token_tree() to output a link by default. I feel like that defeats the purpose of having the two separate theme functions. I can list off the top of my head the few modules that would need to get patched for this. Should we really force it by default?

Dave Reid’s picture

I'd probably be ok with supporting 'dialog' => TRUE if added to theme_token_tree would return the output of theme_token_tree_link instead. This way modules could use the same '#theme' => 'token_tree' but with the newer version of Token it would output the dialog link instead.

DamienMcKenna’s picture

I think providing the better UI by default should be the way to go, then provide an option to revert to the older format if necessary ("more better" by default).

Dave Reid’s picture

Issue tags: +Performance

Adding performance tag.

puddyglum’s picture

Subscribing, this would really help us with #1694720: causes unresponsive script jquery warning

Dave Reid’s picture

Status: Needs work » Needs review
FileSize
5.11 KB

Latest version of the patch. I want to go with this version for now.

Dave Reid’s picture

Status: Needs review » Fixed

Status: Fixed » Closed (fixed)

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

Alan D.’s picture

Nice. Big thanks. We can start removing the themed custom popups that we have been implementing on sites ourselves. It will be interesting to see how these differ.

DamienMcKenna’s picture

FYI here's how I'm using it in the Field Redirection module:

...
  // Show the token browser.
  $form['available_tokens'] = array(
    '#value' => 'Browse available tokens',
    '#theme' => 'token_tree',
    '#token_types' => array($instance['entity_type']),
    '#weight' => 100,
  );

  // Optionally use the new Token 1.2 popup browser.
  if (module_exists('token')) {
    module_load_include('inc', 'token', 'token.pages');
    if (function_exists('theme_token_tree_link')) {
      $form['available_tokens']['#theme'] = 'token_tree_link';
    }
  }
...

Obviously adjust the form spec as necessary.

Dave Reid’s picture

You don't even need to do that. Just do the following and it works regardless of version, but uses the modal if the version supports it.

...
  // Show the token browser.
  $form['available_tokens'] = array(
    '#value' => 'Browse available tokens',
    '#theme' => 'token_tree',
    '#token_types' => array($instance['entity_type']),
    '#weight' => 100,
    '#dialog' => TRUE,
  );
DamienMcKenna’s picture

DaveReid++

Dave Reid’s picture

Correction: '#modal' in #17 should be '#dialog'

rudiedirkx’s picture

I actually don't think this is right...

theme_token_tree_link() takes arguments from the ['#theme' => 'token_tree_link'] element to pass to the link. The link sends them to the page callback to render them using theme_token_tree(). However, theme key token_tree_link doesn't accept all the variables that can be passed to theme key token_tree. For example: token_types.

If you can't pass it to token_tree_link, you can't pass it to token_tree. I've tried with options, but I can't get it into the link's query.

Maybe I'm wrong. Maybe I've been missing something for the last 20 mins =)

rudiedirkx’s picture

Forget it. I wasn't paying attention. I assumed you had to change the #theme key, but you have to add #dialog.

Sk8erPeter’s picture

Shouldn't this issue be marked as "fixed" only if this feature can be enabled without modifying other modules which support Token integration? Or is it the task of the other issue, Token UI 2.0? I know I can also use Token tweaks module until then (btw., thanks for that, too), but I don't really see any updates on this topic since a while. Any plans on that?
Thanks for your efforts!

Dave Reid’s picture

It is up to other modules to use the new option, or for Token tweaks to provide an option to make everything use the dialog by default. There is nothing left to resolve in this issue and it is already closed as fixed.

DamienMcKenna’s picture