In addition to per-preset CSS and Image path settings, should the user be able to specify a global default?

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

tmcw’s picture

The main problem here is that it wouldn't be cleanly exportable: there can either be a global default or per-preset configuration. Mixing the two leads to many cases in which an export & import will change the location of the setting, which can be a dangerous behavior.

davideads’s picture

This doesn't have any effect on exportablity afaict. You are assuming this must be a binary choice, but I don't see any technical reason why it should be.

Let's say a user wants to import a preset: If the person who exported the preset provides image_path and css_path in a preset, then that "wins". Otherwise, the global default should be used. If I import a preset that doesn't specify css_path or image_path and I've set a global default, that should be what is used.

How is this suggestion any different than the current situation -- a default is used, unless there's a preset-based override -- except that it makes the default configurable?

zzolo’s picture

Has been discussed before: http://drupal.org/node/748650

But more discussion is usually awesome. I think this can easily be simple and exportable.

* Use variables, which are exportable as the defined defaults. Probably on the openlayers settings page.
* Use the variable values as the default value, as @davideads suggest.

I would support this.

davideads’s picture

@zzolo -- I didn't realize there was already a thread about this. From the prior thread:

My goal with this would be to add a already collapsed fieldset on the main settings page that provides defaults for some of the presets. This would simply provide a default if one is not set in the form, and fully explain to the user how it works. It is simply a feature of convenience for sites that override the CSS path for every preset.

That's exactly what I'm suggesting.

I'm still confused about where exportability fits here. Are we talking about the exportability of the global defaults? It definitely makes sense to store the default skin/theme/style whatever you want to call it as a variable -- seems like pretty standard practice to me.

strk’s picture

I support this too. Add a proxy setting too, btw (same thing, currently only available at preset level).
@davedeads : have a patch ?

davideads’s picture

@strk -- I've taken to asking before writing patches, and then got caught up with other priorities at work yesterday. I should have a patch some time today.

davideads’s picture

Here's the patch. It implements hook_openlayers_map_preprocess_alter to fold in the defaults, if the defaults are defined and the preset doesn't define them itself. It allows image path, css path, and proxy host to have a global default setting.

davideads’s picture

Argh, how many times have I forgotten to attach a patch to the comment explaining it? This might be my true talent.

zzolo’s picture

Looking good, heres a few points:

  • I would suggest making the fieldset collapsible and collapsed by default
  • The descriptions should be made consistent across the settings.

Wait, there is a bit of complexity here that is not being addressed (though not with exportables). The issue is with the preset form. It is good to say if this value is not set, then use the default. But how do we represent this in the UI form. Currently you can only leave it blank (empty string) which is a valid value, but different form being undefined.

In theory you would want a checkbox that says "Use Default", and it would leave the value undefined and left for the default to decide (also ideally the textfield would get disabled). So, basically the preset form needs to be able to override the use of the default, but explicitly.

Otherwise, this mechanism is great, but how it is represented in the Preset UI needs to be addressed.

davideads’s picture

@zzolo:

Collapsibility: I don't have a strong feeling about this, but why not simply show the options? That form isn't cluttered as it is. Personally, I prefer to know an option is
available without having to hunt for it...

Descriptions consistent: What do you mean? I simply copied the text of the preset options.

Use default checkbox vs. current approach: Another solution might be simply to add help text "leave blank to use default". But you are right -- I thought about this when I was working it up -- specifically the ambiguity in using an empty string as the variable's default value. I'm not convinced the checkbox solution gives you much for the code complexity it would add (which, admittedly, isn't much).

I will revise the patch once we nail these small issues down.

zzolo’s picture

Priority: Normal » Minor

Putting priority down as development capacity on this project has reduced significantly.

ken-g’s picture

Issue summary: View changes
Status: Active » Closed (outdated)