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
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

MrHaroldA’s picture

MrHaroldA CreditAttribution: MrHaroldA at ezCompany commented
Status: Active » Needs review
FileSize
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 CreditAttribution: MrHaroldA at ezCompany 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 CreditAttribution: MrHaroldA at ezCompany commented
FileSize
adminimal_menu-fix_booleans-2513970-4.patch4.22 KB
btopro’s picture

btopro CreditAttribution: btopro commented
FileSize
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 CreditAttribution: Samvel at DrupalJedi commented
Status: Needs work » Needs review
FileSize
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...
Tess Bakker’s picture

Tess Bakker
they/them
Primary language Dutch
CreditAttribution: Tess Bakker at ezCompany commented
Status: Needs review » Fixed

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

Tess Bakker’s picture

Tess Bakker
they/them
Primary language Dutch
CreditAttribution: Tess Bakker at ezCompany commented
Status: Fixed » Closed (fixed)