Feature to toggle widget on and off [#1854184]

Hey there,

I took the liberty of writing a patch to make this module user friendly.

I am a big fan of the concept and I think a module like this is very good for themers who care about responsiveness.

My patch simply adds a toggle checkbox in the theme settings page (at admin/appearance/settings) by which users with the permission to view the widget can also toggle it on and off.

I think this is a basic functionality the module should have. Therefore it can maybe be incorporated in the final version.

Cheers!

Comment	File	Size	Author
#5	added_toggle_to_theme_settings-1854184-5.patch	1.1 KB	willvincent
	windowsize-added-toggle-feature.patch	1.24 KB	upchuk

Comments

Comment #1

alexweber commented 5 December 2012 at 14:35

Status:

Needs review

» Needs work

I was about to suggest this be stored as a regular variable as opposed to a theme setting but, on second thought, this would be nice to be theme-specific. :)

Some minor tweaks:

+++ b/windowsize.moduleundefined
@@ -1,17 +1,30 @@
+function windowsize_form_alter(&$form, &$form_state, $form_id) {

this form alter implementation is probably better off written as windowsize_form_system_theme_settings_alter()

+++ b/windowsize.moduleundefined
@@ -1,17 +1,30 @@
+  if ((user_access('view WindowSize')) && (theme_get_setting('toggle_windowsize') == 1)) {

comparing to == 1 is unnecessary in this case and can be safely omitted

Comment #2

upchuk commented 5 December 2012 at 14:53

Hey there,
Thanks for the comment.

I agree with the first point, can also be written like that.

However, for the second point,

theme_get_setting('toggle_windowsize') == 1

This checks if the the variable in the theme settings is set to 1 (aka the widget is turned on) before adding the javascript and css files.

Thanks!

Comment #3

alexweber commented 5 December 2012 at 17:02

@knob I meant it can be safely replaced with if (theme_get_setting('toggle_windowsize')) there's no need for the explicit comparison operator since PHP will cast to a boolean automatically when performing the comparison. I personally was not a huge fan because it can lead to errors sometimes but its the Drupal way :)