The string 'TRUE' does not check a checkbox, a boolean (or the integer 1) does. The current settings for does not display the default options the module has, so I marked this bug as 'Major'.

This might also explain the weird <strong>Default value => Checked</strong> descriptions?

CommentFileSizeAuthor
#7 adminimal_admin_menu-better-booleans-2513970-7.patch2.76 KBsamvel
#5 adminimal_admin_menu-better-booleans-2513970-5.patch2.76 KBbtopro
#4 adminimal_menu-fix_booleans-2513970-4.patch4.22 KBmrharolda
#1 adminimal_menu-fix_booleans-2513970-1.patch3.24 KBmrharolda

Comments

mrharolda’s picture

mrharolda commented
Status: Active » Needs review
StatusFileSize
new adminimal_menu-fix_booleans-2513970-1.patch3.24 KB

Status: Needs review » Needs work

The last submitted patch, 1: adminimal_menu-fix_booleans-2513970-1.patch, failed testing.

mrharolda’s picture

mrharolda commented
Status: Needs work » Needs review
Related issues: +#2513984: Fix code standard issues

Here's another patch which includes some code style issues; this one should apply on the current codebase, but not when #2513984: Fix code standard issues is fixed.

mrharolda’s picture

mrharolda commented
StatusFileSize
new adminimal_menu-fix_booleans-2513970-4.patch4.22 KB
btopro’s picture

btopro commented
StatusFileSize
new adminimal_admin_menu-better-booleans-2513970-5.patch2.76 KB

updated patch to set all these to FALSE by default. The other issue beyond just 'TRUE' is that out of the box, the module is assuming the value is TRUE even though on the form it is assumed FALSE (because of the 'TRUE' not matching TRUE boolean for the checkbox). This results in a VERY confusing scenario where the form isn't setup to include the jquery / plugin, but the system defaults these to TRUE. So if you go to the form it shows FALSE (even though it's included) then if you save, it'll stop including.

THis patch defaults to FALSE to mirror the previous bad behaviors that was happening anyway. It also prevents 2 versions of jquery from being loaded on systems that use jquery update (which so many do that I dont know this compat thing is even needed at this point).

Status: Needs review » Needs work

The last submitted patch, 5: adminimal_admin_menu-better-booleans-2513970-5.patch, failed testing.

samvel’s picture

samvel commented
Status: Needs work » Needs review
StatusFileSize
new adminimal_admin_menu-better-booleans-2513970-7.patch2.76 KB

Hi guys!

According to #2465471-1: Remove inclusion of jQuery library @ANDiTKO advice to left checkboxes enabled. Attached patch for this
Any way @btopro thank you for the patch, it's can be used if maintainer decided to change logic.

  • Tessa Bakker committed 7561a7c on 7.x-1.x authored by Samvel 
    Issue #2513970 by MrHaroldA, btopro, Samvel: Don't use the string 'TRUE...
tessa bakker’s picture

tessa bakker
she/they
Primary language Dutch
commented
Status: Needs review » Fixed

Applied patch #7 if still needed a follow up can be created.

tessa bakker’s picture

tessa bakker
she/they
Primary language Dutch
commented
Status: Fixed » Closed (fixed)