The LoginToboggan settings form is looooooooooonnnng. It could be only looooooong if we made all of those t('Enabled')/t('Disabled') settings into checkboxes instead.

CommentFileSizeAuthor
#2 logintoboggan-radios-to-checkboxes-264300-2.patch14.65 KBwebchick

Comments

hunmonk’s picture

hunmonk commented

it's not long, it's feature-full... :P

seems like a reasonable suggestion. how's about a patch? :)

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Status: Active » Needs review
StatusFileSize
new logintoboggan-radios-to-checkboxes-264300-2.patch14.65 KB

This was pretty straight-forward, except site 403 is doing something a bit wacky with its Enabled/Disabled options; it's storing the path to the 403 page, so I re-worded it a bit so instead of choosing from "Enabled vs. Disabled" it's now "Leave as site default vs. Show login form on access denied page."

Additionally, "Set password" and "Immediate login" were a bit terse for as checkboxes, so I expanded those out to "Allow users to set their own password" and "Allow users to login immediately upon registering", as that matches more closely with the "Allow users to login using their e-mail address" which already existed. The help text (which looks out of date) has been updated accordingly.

Also, as I was going through, that 'version' thing at the top is a neat trick but probably unnecessary now that we have the module versions displayed right on the module page, no? It might be confusing that admin/build/modules says they have 6.x-1.0 and this page says they have 1.137. Just a thought. I left it in though, since it doesn't really have anything to do with this patch.

hunmonk’s picture

hunmonk commented
Status: Needs review » Postponed (maintainer needs more info)

just curious, did you check to make sure that all the tests performed against these set variables in the module still work the same? i'm not sure if checked/unchecked gets stored in the variables table in exactly the same way as those radio buttons...

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented

Nope, because it would take me just as long to test all of the options as it would to write a bunch of simpletests so that no human being ever had to click on all of these options. And unfortunately, I have time for neither.

I just code 'em. It's up to the reviewers to review 'em. ;)

hunmonk’s picture

hunmonk commented
Status: Postponed (maintainer needs more info) » Fixed

applied to HEAD, which looks like it will be 6.x-2.x, with a very minor tweak to a description.

Anonymous’s picture

Anonymous (not verified) commented
Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for two weeks with no activity.