At admin/settings/statistics a number of radios are shown to enable or disable certain logs. These should be replaced by checkboxes.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Xano’s picture

I might have explained my reasons for submitting this issue a bit more extensively:

  1. The settings this issue addresses are about enabling a certain feature, saying yes or no to something. Currently, the radios' labels do not add any information that hasn't already been given. Also, according to Jakob Nielsen A stand-alone checkbox is used for a single option that the user can turn on or off. (source)
  2. It takes up less space on the page.
  3. It requires less code.
Xano’s picture

Assigned: Unassigned » Xano

Just figured "Why not?"

Dave Reid’s picture

I'm agreed these options will probably cleaner as checkboxes.

Xano’s picture

Assigned: Xano » Unassigned

Okaaaaay, wrong issue (A). Do intend to work on it, just haven't started yet.

Damien Tournoud’s picture

There is not need to duplicate the component attribute.

Xano’s picture

What do you mean?

Dave Reid’s picture

Title: Enabled/Disabled radios should be checkboxes » Clean up statistics admin settings
Status: Active » Needs review
FileSize
2.67 KB
Dave Reid’s picture

Fixed closing array indentation.

Status: Needs review » Needs work

The last submitted patch failed testing.

Dave Reid’s picture

Status: Needs work » Needs review

I don't believe you testing bot! :)

Status: Needs review » Needs work

The last submitted patch failed testing.

Dave Reid’s picture

Status: Needs work » Needs review

Dear Testing Bot,

I really don't believe you. This patch has nothing to do with the 'Field form tests.'

Regards,
Dave

Xano’s picture

I guess you did it. The bar looks bright green over here :D

Bojhan’s picture

Do you have a screenshot?

Xano’s picture

cburschka’s picture

Status: Needs review » Reviewed & tested by the community

Very nice code-style and comment fix along with those checkboxes.

I see nothing that could be improved in this patch, and it seems Testbot finally likes it as well.

webchick’s picture

Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs screenshots

Heh, #14 was asking for a before/after screenshot of the feature, not testing bot. ;) Let's add this, and additionally:

+  // Access log settings

Comments start with a capital letter, end with a period. This applies throughout the patch.

   $form['access'] = array(
     '#type' => 'fieldset',
+    '#title' => t('Access log settings')
+  );

The final index in an array should end with a comma as well -- so t('Access log settings'),). It looks like a mistake but isn't. This applies throughout the patch.

cburschka’s picture

+ '#title' => t('Access log settings')

Ack, I need new glasses! =D

Dave Reid’s picture

Status: Needs work » Needs review
FileSize
2.92 KB

Revsied patch. Thanks for catching those, not sure why I missed those! *slaps face*

Stefan Nagtegaal’s picture

Status: Needs review » Reviewed & tested by the community

Nice one!
tested and verified to work on my D7 install.. Pretty RTBC.. Lets commit this webchick! :-)

catch’s picture

Status: Reviewed & tested by the community » Needs review

Still needs before/after screenshots.

mrfelton’s picture

FileSize
33.17 KB
38.59 KB

Here is a before and after screenshot. This is the correct way to do it for simple on/off settings like this. +1

Bojhan’s picture

Status: Needs review » Reviewed & tested by the community

Looks good, marking RTBC

Xano’s picture

Can't we get rid of the fieldets as well?

webchick’s picture

Status: Reviewed & tested by the community » Fixed

Now that's much better. Committed to HEAD. Thanks! :)

Stefan Nagtegaal’s picture

We do this in *a lot* of UI-places in drupal. For example, the whole Create node type thing is full of the "wrong" usage of UI-elements.
I am pretty sure, I filed a bug report/feature request for this using proper forms years ago.

Unfortunatly I can not find it anymore...

sun’s picture

This is the first committed core patch without failures I am looking at today. Thank you!

Status: Fixed » Closed (fixed)

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