Patch does several things (see snap shot so it will make sense):

- Change settings key to be with underscore -- so it acts nicely with form API (in $form_state['values'] - the values can't be with space).
- Add theme-settings.php
- Add ninesixty_theme_get_default_settings() that is a shameless copy from Zen :)
- Add a default value to '960_debug'.

Zen-960 will use it like so:


/**
 * Implementation of THEMEHOOK_settings() function.
 *
 * @param $saved_settings
 *   An array of saved settings for this theme.
 * @return
 *   A form array.
 */
function zen_ninesixty_settings($saved_settings) {
  // Get the default values from the .info file.
  $defaults = zen_theme_get_default_settings('zen_ninesixty');

  // Merge the saved variables and their default values.
  $settings = array_merge($defaults, $saved_settings);

  $form = array();

  // Add Zen settings.
  $form += zen_settings($saved_settings, $defaults);

  // Remove some of the base theme's settings.
  // We don't need to select the base stylesheet, as we are already using 960.
  unset($form['themedev']['zen_layout']);

  // Add Ninesixty settings.
  $form += ninesixty_settings($saved_settings, $defaults);

  // Return the form
  return $form;
}

Surprisingly enough - it works ;)

CommentFileSizeAuthor
#1 960-theme-settings-2.patch5.21 KBamitaibu
960-theme-settings-1.patch4.47 KBamitaibu

Comments

amitaibu’s picture

amitaibu
Primary language Hebrew
Location Israel
commented
StatusFileSize
new 960-theme-settings-2.patch5.21 KB

Re-roll, as I accidentaly worked with out-dated version.

dvessel’s picture

dvessel commented
Status: Needs review » Fixed

Doh! Good call. My working copy is already different from dev. I'll get this in there.

dvessel’s picture

dvessel commented
Status: Fixed » Active

I just noticed in the code that a few things don't make sense. For instance, there are extra parameters for ninesixty_settings that are never used and the name of the function can be phptemplate_settings which will allow the settings form to be inherited into sub-themes. This ability looks like an opportunity to simplify the theme api settings for subthemes.

http://api.drupal.org/api/function/system_theme_settings/6

The patch looks like a direct copy from Zen and I don't understand all the reasons why it was made like that. Maybe legacy stuff that doesn't have to apply here? I'd like to improve on it before committing. I'll get a patch up.

amitaibu’s picture

amitaibu
Primary language Hebrew
Location Israel
commented

> Maybe legacy stuff that doesn't have to apply here?

To what code do you refer?

dvessel’s picture

dvessel commented

I looked over the code again and it's not really legacy but I don't like how it handles the theme settings form. I figured out an improved way.

As it is now, each sub-theme must re-implement theme-settings.php even if it doesn't add anything new. There's a way to create theme-settings.php only in the base theme and have it apply to all sub-themes. Only when the sub-theme adds a new feature will it have to code for it and when it does, it will be simpler than adding a whole new theme-settings.php file. All it will have to worry about is adding it's own specific features leaving the rest the the base theme.

Sorry this is taking a while but I've been adding a bunch of big features to v.2 to simplify development even though it has a lot more code.