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.
API page: http://api.drupal.org/api/drupal/core--modules--system--system.module/fu...
Describe the problem you have found:
This has pretty tangled logic which the docs don't cover at all.
Comment | File | Size | Author |
---|---|---|---|
#12 | 1394790-12.drupal.system_admin_compact_mode-missing-docs.patch | 1.03 KB | joachim |
#1 | 1394790.drupal.system_admin_compact_mode-missing-docs.patch | 743 bytes | joachim |
Comments
Comment #1
joachim CreditAttribution: joachim commentedHere's a patch.
Comment #2
jhodgdonSeems like a good thing to fix, thanks!
The documentation in the patch needs some cleanup:
- All lines should wrap as close to 80 characters as possible without going over.
- Needs some copy editing (grammar etc.).
- "ie" should be "i.e.,".
- "UI" should be written out as "user interface".
It's also actually not accurate to say there is no user interface for this. There is a path for it (admin/compact), so if someone visits admin/compact/on or admin/compact/off, it will turn compact mode on/off. theme_system_compact_link() makes these links, and this is called in several places in the code.
Comment #3
joachim CreditAttribution: joachim commentedadmin/compact sets the cookie; it doesn't set the variable.
Comment #4
jhodgdonTrue. But the patch says "This is hidden, ie there is no UI to control it,...", without explaining whether "this" refers to the cookie or the variable. I read it to say that there was no user interface for anything related to this function.
So, I don't think it would hurt to be clear, or to point out the UI for the cookie, or at least @see the related functions. Right?
Comment #5
sven.lauer CreditAttribution: sven.lauer commentedWhile we are improving the docs for this function, wouldn't it be nice to explain what compact mode _is_?
Comment #6
jhodgdonYes, that too.
Comment #7
joachim CreditAttribution: joachim commentedShan't bother with a patch while we refine the text:
Comment #8
jhodgdonLooks pretty good...
A couple of grammar/usage/typographical nitpicks:
- "i.e.", as you are using it, needs a comma after it. Or "i.e." could just be left out without changing the meaning in any way, I think?
- "Determines if" -> "Determines whether"
Also, maybe this explanatory text should mention that if the admin_compact_mode variable is not set, it defaults to FALSE?
Comment #9
jhodgdonforgot status change on previous comment
Comment #10
joachim CreditAttribution: joachim commentedComment #11
jhodgdonThat looks reasonable - I think it's ready for a patch. Thanks!
Comment #12
joachim CreditAttribution: joachim commentedDone.
Comment #13
jhodgdonThanks, looks good to me!
Comment #14
Dries CreditAttribution: Dries commentedCommitted to 8.x and back-ported to 7.x.
Comment #16
cweagansUpdating tags per http://drupal.org/node/1517250