Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
At admin/settings/statistics a number of radios are shown to enable or disable certain logs. These should be replaced by checkboxes.
Comment | File | Size | Author |
---|---|---|---|
#22 | Statistics-before.png | 38.59 KB | mrfelton |
#22 | Statistics-after.png | 33.17 KB | mrfelton |
#19 | 362852-statistics-admin-form-cleanup-D7.patch | 2.92 KB | Dave Reid |
#8 | 362852-statistics-admin-form-cleanup-D7.patch | 2.92 KB | Dave Reid |
#7 | 362852-statistics-admin-form-cleanup-D7.patch | 2.67 KB | Dave Reid |
Comments
Comment #1
XanoI might have explained my reasons for submitting this issue a bit more extensively:
Comment #2
XanoJust figured "Why not?"
Comment #3
Dave ReidI'm agreed these options will probably cleaner as checkboxes.
Comment #4
XanoOkaaaaay, wrong issue (A). Do intend to work on it, just haven't started yet.
Comment #5
Damien Tournoud CreditAttribution: Damien Tournoud commentedThere is not need to duplicate the component attribute.
Comment #6
XanoWhat do you mean?
Comment #7
Dave ReidComment #8
Dave ReidFixed closing array indentation.
Comment #10
Dave ReidI don't believe you testing bot! :)
Comment #12
Dave ReidDear Testing Bot,
I really don't believe you. This patch has nothing to do with the 'Field form tests.'
Regards,
Dave
Comment #13
XanoI guess you did it. The bar looks bright green over here :D
Comment #14
Bojhan CreditAttribution: Bojhan commentedDo you have a screenshot?
Comment #15
XanoClicketyclick.
Comment #16
cburschkaVery 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.
Comment #17
webchickHeh, #14 was asking for a before/after screenshot of the feature, not testing bot. ;) Let's add this, and additionally:
Comments start with a capital letter, end with a period. This applies throughout the patch.
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.
Comment #18
cburschkaAck, I need new glasses! =D
Comment #19
Dave ReidRevsied patch. Thanks for catching those, not sure why I missed those! *slaps face*
Comment #20
Stefan Nagtegaal CreditAttribution: Stefan Nagtegaal commentedNice one!
tested and verified to work on my D7 install.. Pretty RTBC.. Lets commit this webchick! :-)
Comment #21
catchStill needs before/after screenshots.
Comment #22
mrfelton CreditAttribution: mrfelton commentedHere is a before and after screenshot. This is the correct way to do it for simple on/off settings like this. +1
Comment #23
Bojhan CreditAttribution: Bojhan commentedLooks good, marking RTBC
Comment #24
XanoCan't we get rid of the fieldets as well?
Comment #25
webchickNow that's much better. Committed to HEAD. Thanks! :)
Comment #26
Stefan Nagtegaal CreditAttribution: Stefan Nagtegaal commentedWe 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...
Comment #27
sunThis is the first committed core patch without failures I am looking at today. Thank you!