search

backdrop-contrib / css_injector

Allows administrators to inject CSS into the page output based on configurable rules
GNU General Public License v2.0
0 stars 1 forks source link

Default value condition never met - results in "always-on" #13

Closed indigoxela closed 3 months ago

indigoxela commented 4 months ago

I don't think, this condition works as expected:

https://github.com/backdrop-contrib/css_injector/blob/1.x-1.x/css_injector.module#L159

As config_get('css_injector.settings', 'css_injector_use_ace') evaluates to false, if the checkbox is unchecked (value is 0), we always end up with the default value being on. So it's impossible to turn the Ace editor off.

indigoxela commented 4 months ago

Here's a pull request @vstemen : https://github.com/backdrop-contrib/css_injector/pull/14

olafgrabienski commented 4 months ago

Interesting, when I disable the "Use Ace syntex highlighter" (note the typo) setting at admin/config/development/css-injector/settings and save the configuration, the checkbox remains enabled, but when editing a CSS rule, the highlighter gets disabled. Screenshot:

edit / PS: Once trying to disable the setting, I can't re-enable it via the user interface.

olafgrabienski commented 4 months ago

After applying the PR on my test site, the general setting works fine. And with the PR it's also possible to toggle the syntax highlighter of single CSS rule forms.

indigoxela commented 4 months ago

... when editing a CSS rule, the highlighter gets disabled. Screenshot:...

Yes, saw that, too. Obviously this was never tested (even with D7?). So I guess, JS handling with the global setting "off" should get fixed, too. Probably in a different issue, as this requires a closer look and changes in JS. Or even the expected behavior needs to get defined (let people still toggle, or not...).

vstemen commented 4 months ago

Sorry about not being active lately. Unfortunately I am not setup to work on backdrop right now. I plan to get another computer setup before too long for working on it. Hopefully @klonos will be able to pick up the slack in the mean time.

See https://github.com/backdrop-contrib/css_injector/issues/5#issuecomment-2136373196

klonos commented 3 months ago

Thanks @indigoxela and @olafgrabienski 🙏🏼 ...merged.

(apologies for the unresponsiveness)