The attached patch does a few things:

  • Add a theme setting for skip link text, so people can easily customize without overriding html.tpl.php.
  • Call it 'skip link' by default instead of 'jump link'. I believe skip link is a more recognisable term to have as a default, and the settings allow people to easily adjust the text into something like 'jump' if they want to.
  • Use 'href' for the variable name instead of 'target'. I think target would kind of conflict with the target HTML attribute, and because the variable is only seen in the code (not in the UI), href might be more clear and suitable.
  • Synchronize the variable descriptions in html.tpl.php with the ones for the UI a little bit.
  • Add a link to http://drupal.org/node/467976 in the setting description, for people who really don't know what a skip link is yet.

What do you think of these?

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

maartenverbaarschot’s picture

Might want to include that link as a comment in the .info file too btw... I'll await feedback first.

barraponto’s picture

I guess it should be done in template.php, without touching the html.tpl.php file.

maartenverbaarschot’s picture

l() is a nice addition.

Still I think it wouldn't make sense having to dig into template.php to just customize the link text when there's a setting for the href. They should be either both settings, or both in template.php.

barraponto’s picture

Sure, let's merge the approaches and expose them in theme-settings.

maartenverbaarschot’s picture

Added $_GET['q'] as second parameter for l() and updated variable documentation in html.tpl.php.

maartenverbaarschot’s picture

Status: Active » Needs review
KrisBulman’s picture

nice addition! but patch fails for me

$ git clone --branch 7.x-3.x http://git.drupal.org/project/zen.git
$ cd zen
$ wget http://drupal.org/files/issues/zen-skip-link-settings-3.patch
$ git apply zen-skip-link-settings-3.patch 
zen-skip-link-settings-3.patch:30: trailing whitespace.
  
warning: template.php has type 100755, expected 100644
warning: 1 line adds whitespace errors.
maartenverbaarschot’s picture

Right… (Reminder to self: don't indent empty lines :))

KrisBulman’s picture

i can confirm that the patch does the trick, i applied it, created a new subtheme (via the new drush command).. checked and changed new theme settings, then verified changes through firebug (screenshot below)

https://skitch.com/krisbulman/fn1a8/local-dev-environment-drupal-7

KrisBulman’s picture

Status: Needs review » Reviewed & tested by the community
JohnAlbin’s picture

Version: 7.x-3.x-dev » 7.x-5.x-dev
Status: Reviewed & tested by the community » Patch (to be ported)
FileSize
4.65 KB

You can't pass a variable into the t() function. It requires a string. The i18n module handles converting strings input via an admin form.

Given that, I don't really see the advantage of making the link a variable containing un-alterable HTML snippet.

I do, however, like the idea of making the link text a setting. How about we just print out the setting in the html.tpl.php like we do for the anchor link?

I've committed this patch for 7.x-5.x.

JohnAlbin’s picture

Version: 7.x-5.x-dev » 7.x-3.x-dev
FileSize
4.07 KB

We can't rename the zen_jump_link_target variable in Zen 7.x-3.x, so I had to name the new variable to match that naming convention. :-\

JohnAlbin’s picture

Status: Patch (to be ported) » Fixed

Status: Fixed » Closed (fixed)

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