Hi Guys,

While I considered marking this as a bug, it is quite possible that it is a feature:

When I click on the a token in the Token Tree (to assign it to my selected field) it triggers a scroll to top of field action, which can be quite agitating. I understand that if the field is off screen it may be necessary, but this is occurring with the field just above the token tree, perfectly visible.

Would it be possible to turn this behavior off completely, or if not at least restrict it to only scroll if the field is off screen (which could still be as annoying for some users, even more annoying if you're trying to insert multiple tokens but the browser keeps scrolling away from the token tree.)

 

Otherwise, nice work on it, a great improvement to the standard token help.

Cheers,
Deciphered.

CommentFileSizeAuthor
#13 token-scroll_on_insert_optional-918892-13.patch1.39 KBAndrew Answer
#7 scroll_on_insert_optional.patch2.27 KBtstoeckler
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Dave Reid’s picture

Dave Reid
he/him
Primary language English
Location Nebraska USA
CreditAttribution: Dave Reid commented
Title: Token Tree - Don't scroll browser on click » Allow users to disable the 'scroll to textfield' when clicking a token in the tree UI

I'm not sure how it would be done. Maybe there should be some kind of clickable link at the top of the table to 'Turn off' that feature?

Dave Reid’s picture

Dave Reid
he/him
Primary language English
Location Nebraska USA
CreditAttribution: Dave Reid commented
Version: 6.x-1.14 » 7.x-1.x-dev
Deciphered’s picture

Deciphered CreditAttribution: Deciphered commented

Yeah, making it an option does complicate matters, personally I'd like the functionality completely removed, but that's not the Drupal way and I'm sure there are a lot of people who disagree with me and like the feature.

There are a few options that I can think of the top of my head:
1. As you said, have a clickable link at the top (or bottom) of the Token Tree to toggle the funcionality.
2. Same as above but with a checkbox instead of link.
3. Add another argument in the theme('token_tree') callback to toggle this functionality on, off, on by default (with option) or off by default (with option).
4. Have a variable for the behavior that can set in settings.php

My personal preference is to #3, and if necessary to make it toggleable by the user, have #3 in combination with #2.

Dave Reid’s picture

Dave Reid
he/him
Primary language English
Location Nebraska USA
CreditAttribution: Dave Reid commented

#3 is already implemented to be able to disable or enable it completely.

Deciphered’s picture

Deciphered CreditAttribution: Deciphered commented

Maybe I'm missing something, but it appears to me that you can disable click-insert, not the scrolling.

I have nothing against click-insert, it's a must in my opinion, it's the scrolling ($('html,body').animate({scrollTop: $(myField).offset().top}, 500);) that I object to.

Deciphered’s picture

Deciphered CreditAttribution: Deciphered commented

Quick video example of why I'm requesting this: http://www.youtube.com/watch?v=dCVFaC-ZtmQ :)

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
CreditAttribution: tstoeckler commented
Status: Active » Needs review
FileSize
scroll_on_insert_optional.patch2.27 KB

I really don't get what the purpose of this scrolling is, I also find it very frustrating and unnatural.

Anyways, here is a small patch that introduces a new attribute: '#scroll_on_insert' which controls this behaviour. It defaults to TRUE, so nothing is changed by default. But you can now set #scroll_on_insert => FALSE and the scrolling stops.
Don't forget to clear the cache when trying it out.
I tested it out manually and it seems to work.
I'm not really that good with jQuery, though, so, while it works, I bet there's a nicer way to do the if().

Deciphered’s picture

Deciphered CreditAttribution: Deciphered commented
Status: Needs review » Needs work

Hi tstoeckler,

Nice to finally get some support for this.

Looked over the patch, and my main concern is that introducing a new argument to a public function and not putting it at the end means that anyone already defining it will likely have problems. Either it goes at the end, or it could possibly become part of click_insert (ie, 0 = FALSE/disabled completely, 1 = TRUE w/Scroll, 2 = Insert w/o Scroll).

Just my 2c, hope this does get accepted though as the scrolling is just a pain.

Cheers,
Deciphered.

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
CreditAttribution: tstoeckler commented

@Deciphered: That's not true, the arguments get passed to the theme function as a single $variables parameter. Only functions calling theme_token_tree() directly, in contrast to theme('token_tree'), and don't set $variables['scroll_on_insert'] will see PHP notices. But I don't know a use-case for that, and it's also just a notice, not a WSOD or something.

Anyway, would be great to hear back from Dave Reid or one of the other maintainers. I would gladly adjust the patch so that it works like you describe in #8.

Deciphered’s picture

Deciphered CreditAttribution: Deciphered commented
Status: Needs work » Needs review

Actually, I think you're right (blame it on reviewing code first thing in the morning), where it not keyed it would be another matter.

Will try to get some time to run a test with the patch later on today.

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
CreditAttribution: tstoeckler commented
Assigned: Unassigned » tstoeckler

Assigning to me, also bump.

Chris Matthews’s picture

Chris Matthews CreditAttribution: Chris Matthews as a volunteer commented
Assigned: tstoeckler » Unassigned
Issue summary: View changes
Status: Needs review » Needs work
Issue tags: +Needs reroll

The 8 year old patch in #7 to token.module and token.pages.inc does not apply to the latest token 7.x-1.x-dev and if still applicable needs reroll.

Checking patch token.js...
Hunk #1 succeeded at 75 (offset 29 lines).
Checking patch token.module...
error: while searching for:
      'file' => 'token.pages.inc',
    ),
    'token_tree' => array(
      'variables' => array('token_types' => array(), 'global_types' => TRUE, 'click_insert' => TRUE, 'show_restricted' => FALSE, 'recursion_limit' => 4),
      'file' => 'token.pages.inc',
    ),
  );

error: patch failed: token.module:121
error: token.module: patch does not apply
Checking patch token.pages.inc...
error: while searching for:
    'empty' => t('No tokens available.'),
  );
  if ($variables['click_insert']) {
    $table_options['caption'] = t('Click a token to insert it into the field you\'ve last clicked.');
    $table_options['attributes']['class'][] = 'token-click-insert';
  }

  return theme('tree_table', $table_options);

error: patch failed: token.pages.inc:90
error: token.pages.inc: patch does not apply
Andrew Answer’s picture

Andrew Answer CreditAttribution: Andrew Answer as a volunteer commented
Status: Needs work » Needs review
Issue tags: -Needs reroll
FileSize
token-scroll_on_insert_optional-918892-13.patch1.39 KB

Patch re-rolled.