Theme specific configurations always override the global configuration

Hi joelpittet,

Thanks for getting back to me. I agree that option 2 is a big change and might break more than we'd like.

so maybe concentrate on fallbacks based on empty values in theme specific settings?

That is unfortunately part of the problem. Most of the theme features are checkboxes which are either TRUE or FALSE, but never empty. (False is not the same as empty as the user should be able to disable a feature for a specific theme.)

So IMO the only solution would be to have 3 radio buttons per feature: enabled, disabled and global, with the respective values TRUE, FALSE and NULL. I'll see if I can create a patch for that, unless you have any other ideas?

It could look something like this:

Obviously, for the global theme settings form the last value should be cut off.

I guess this could be a rough idea for the patch. I have tried to keep the types for the form configuration the same (booleans) so casting them to integers during the form building process. Setting a value to 'global' will explicitly clear it from the theme specific configuration and theme_get_setting() will inherit its value from the global theme. (After saving the form, all global values inherit the default config via theme_get_setting and display either 'enabled' or 'disabled'. This will be fixed in #2402467: Prevent theme settings configuration overrides from bleeding into stored configuration.)

And while writing this comment I realize that I am explicitly clearing feature-configuration items, so it probably does not yet work nicely for default logo and image.

I wonder how base theme settings will compound this problem?

Nothing in the scheme definition is changed, so defining TRUE of FALSE will enable or disable the feature. Omitting it will make it assume global. In the worst case a user will have to edit the configuration and revert it to global. It would allow themers to ship their theme with sensible defaults, so I foresee not many problems.

Finally some time to have another look at this. Lets see what tests need fixing as of 8.5.

This should make the tests pass.

Great, now there are some minor UI issues left to iron out.

I believe this covers it all. Tagging for usability review to hear feedback on the radios.

Not sure if this requires a change record. The functional test had to be adjusted from TRUE/FALSE to 1/0 so they comply with the options allowed values list. I cannot imagine there will be anybody posting values directly to the form. (Boolean values written directly to configuration will remain functioning.)

Styling of radio's is achieved after implementing #2236789: Styling of inline radios broken: inappropriate trailing colons (breaks EditorImageDialog).
This patch should also resolve #2402467: Prevent theme settings configuration overrides from bleeding into stored configuration.

I am upping this to a major bug. It is not completely broken, but certainly undesired behavior and clearly not functioning as it should.

@Bojhan, I am not really getting what you mean. By just using checkboxes you can only have 2 sates (enabled/disabled). This is a problem because if you override someting at the theme-specific level, there is no way to revert that setting to global.

To replicate:
I set my logo to show for all themes (global config).
Then in one theme, I disable the logo.
Then in the same theme I enable it again.
(so far so good)
Now I disable it globally, and the theme specific implementation will persist.

Adding some indication that the value is overridden is a possibility, but it will not allow a user to 'reset' a theme to follow the global defined settings.

There is a need for more than 2 states.

Can we not assume, when a user selects the global default - that this is the the desired state?

Yes, that makes sense.

And to only allow for "overriding", whereas inheriting is implied when the user selects the same option?

Not per sé. There might be themes where you want to explicitly disable a certain function (ugly logo implementation so you do it in a block or something). If you were to change some feature and save the form, you could end up setting others to global without wanting to.

Checkboxes can be used in 2 ways;
Enabled and empty
or
Explictly enabled and Explicitly disabled

In the first case it would not matter much because if the checkbox is empty we can skip its value during submission and inherit global as we please. However on the theme configuration page they are used to explicitly enable and explicitly disable a feature.
I have thought of adding an inline link behind the checkbox to 'revert to global', but even then, the next time the form is submitted the empty checkboxes would happily define new overrides.

Using 2 checkboxes (one to 'enable' the override and one for the feature) would seem bad UX as well. That is why I ultimately ended up with the radios.

As long as there is no alternative to the UI, I do personally do not know how to proceed here.
IMO the theme specific pages need an option to revert to the default setting, but as Bojhan explained in #25 these need to be checkboxes. But then you simply miss a state (explicit enabled for this theme, explicit disabled for this theme, inherit default)

@shaktik please try to understand the issue before writing code. We currently do not need a patch, but somebody has to decide on the design instead. As Bojhan explained in #25 we cannot use radio's because of the UI standards. As long as that is an issue, all your effort will be wasted because it will not be used.

Not sure if the product manager is the right person here, but we're stuck. #25 more or less states that checkboxes are needed for a 3-state situation. It would be nice if somebody could make a choice here so that we can progress with #2402467: Prevent theme settings configuration overrides from bleeding into stored configuration as well.