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?

Comments

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

StatusFileSize
new1.22 KB

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

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.

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

StatusFileSize
new4.97 KB

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

Status:Active» Needs review

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.

StatusFileSize
new4.97 KB

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

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

Status:Needs review» Reviewed & tested by the community

Version:7.x-3.x-dev» 7.x-5.x-dev
Status:Reviewed & tested by the community» Patch (to be ported)
StatusFileSize
new4.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.

Version:7.x-5.x-dev» 7.x-3.x-dev
StatusFileSize
new4.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. :-\

Status:Patch (to be ported)» Fixed

Status:Fixed» Closed (fixed)

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