Automattic / gutenberg-ramp

Control conditions under which Gutenberg loads - either from your theme code or from a UI
https://wordpress.org/plugins/gutenberg-ramp/
76 stars 15 forks source link

Options for Loading Function are Very Confusing #38

Closed mikeselander closed 6 years ago

mikeselander commented 6 years ago

I've been working with enabling this on a client's site and am a bit confused by how these parameters are supposed to work. They don't follow expected behavior for how booleans should work, which leads to unexpected results.

Consider the following values: wpcom_vip_load_gutenberg( true ); -> does not load wpcom_vip_load_gutenberg( false ); -> loads wpcom_vip_load_gutenberg( [ 'load' => true ] ); -> does not load wpcom_vip_load_gutenberg( [ 'load' => 1 ] ); -> loads

Passing false into a function that determines whether G'berg is supposed to load forces it to load, but true turns it off? Additionally, the type checking on the load value is so strict (and I rarely disagree with strict type checking) that 'load' => true doesn't even work when true should be the value that does load G'berg instead of the other way around.

Additionally, when testing various values the difference between [ 'load' => 1 ] and [ 'load' => true ] is easily mistaken due to the storage of this as an option instead of directly respecting the function's value. If you change it and reload the page, the new value has no immediate effect which leads to false positives and negatives.

maevelander commented 6 years ago

Resolved by https://github.com/Automattic/ramp-for-gutenberg/pull/48 which introduces booleans as acceptable arguments to the helper function.