Replace "Selected" dropdown with checkbox in category edit form

Yes :)

The screenshot with a selected checkbox and that description looks quite self-referential :) Initially it didn't seem to mean that much. Maaaaybe a bit more specific:

[x] Make this the default category.

That two-line description for Weight looks bad doesn't it? #1477802: Shorter description for contact category weight

Modified the title as suggested in #5.

Modified contact.test to assign TRUE or FALSE to $edit['selected'], instead of '0' or '1', as suggested by DrupalWebTestCase::drupalPost $edit parameter:
"A checkbox can be set to TRUE to be checked and FALSE to be unchecked."

Also changed the issue title, as "Selected" was spelled "Seleced".

I'm worried that contact_category_edit_form_submit() might unset defaults with integer zero first, then write records with boolean zero (if FAPI doesn't change it back to 0 and 1).

Actually only unset value should be converted to FALSE according to WebTestBase::handleForm()
So I changed comment for drupalPost()

case 'checkbox':
          // To prevent checkbox from being checked.pass in a FALSE,
          // otherwise the checkbox will be set to its value regardless
          // of $edit.
          if ($edit[$name] === FALSE) {
            unset($edit[$name]);
            continue 2;
          }
          else {
            unset($edit[$name]);
            $post[$name] = $value;
          }
          break;

Test and documentation should backported

Looks good to me

Related UX issue #599770: Clean up the contact forms listing UI: Allow to set the default category and weights on the listing page

Thanks. Committed/pushed to 8.x.

This breaks string freeze and changes the UI, I don't think this is backportable.

let's get commiter's review probably this could be backported

I only moved it for backport because of:

Test and documentation should backported

Don't know about the change itself (although that's also not up to me :).

I mean this

You can't change the test without the changes to contact.admin.inc. I'll make a try...

Sorry, forgot the Status. *blush*

Edit: And didn't know that I don't have to re-attach the patch again.

Well, let's hope for the best... after all, it's my birthday today. :-)

So now this is just the patch in #10, but without the test docs changes. But still with the string changes.

And, it changes the form structure, possibly breaking any custom form_alters. See http://drupal.org/node/1527558 for an explanation of why this isn't backportable. And http://groups.drupal.org/node/210973#comment-701613 lists this as the second most intrusive type of Form API change, after removing an element completely.

Regarding http://groups.drupal.org/node/210973#comment-701613.

Well written, but if you read the last part of that comment... my conclusion is that this issue to be considered as back-portable?

So, here is a patch with the missing changes to the documentation of simpletest. And yes, it's a copy of #10... but this works for Drupal7. :-)

Fixing tag.

Let's get commiters feedback, Summary updated

Hm. I also don't know that this is back-portable patch, but it's worth running past David. However, David has requested help on resolving release blockers, so I don't feel comfortable committing feature patches until that happens.

Sorry for not providing feedback here sooner, but I didn't have much time to work on Drupal the last few weeks, and I was focusing most of the time I did have on the release blockers (which I was only getting help from a couple other people on) and other bugs.

I would lean against committing this to Drupal 7 also. The policy linked to above makes clear that changing #type is a relatively intrusive change, and that we generally only consider these for critical issues.... In practice I think it's often more like "issues which have a big impact on user experience" rather than actual critical issues. But either way, does this issue qualify for that? Following user interface guidelines is of course important, but absent any direct evidence that the current behavior is causing usability issues for people trying to use this particular form, I think we should probably leave it alone.

Moving back to "needs review" to further discuss if that's the case, and if particular parts of the patch (e.g., documentation) should be backported instead.

I agree with that - a #type change can have bad consequences on existing sites.