Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
In addition to per-preset CSS and Image path settings, should the user be able to specify a global default?
Comment | File | Size | Author |
---|---|---|---|
#8 | 910160-openlayers-global-defaults.patch | 4.78 KB | davideads |
Comments
Comment #1
tmcw CreditAttribution: tmcw commentedThe 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.
Comment #2
davideads CreditAttribution: davideads commentedThis 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?
Comment #3
zzolo CreditAttribution: zzolo commentedHas 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.
Comment #4
davideads CreditAttribution: davideads commented@zzolo -- I didn't realize there was already a thread about this. From the prior thread:
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.
Comment #5
strk CreditAttribution: strk commentedI support this too. Add a proxy setting too, btw (same thing, currently only available at preset level).
@davedeads : have a patch ?
Comment #6
davideads CreditAttribution: davideads commented@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.
Comment #7
davideads CreditAttribution: davideads commentedHere'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.Comment #8
davideads CreditAttribution: davideads commentedArgh, how many times have I forgotten to attach a patch to the comment explaining it? This might be my true talent.
Comment #9
zzolo CreditAttribution: zzolo commentedLooking good, heres a few points:
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.
Comment #10
davideads CreditAttribution: davideads commented@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.
Comment #11
zzolo CreditAttribution: zzolo commentedPutting priority down as development capacity on this project has reduced significantly.
Comment #12
ken-g CreditAttribution: ken-g as a volunteer and commented