Look around in Drupal6 and you'll find three different ways hierarchical lists are rendered in selectboxes:
- double-hyphen+space indentation &
"<None>"
option (menu parent item in node edit form) - single-hyphen indentation &
"<none>"
option (menu parent item & related terms in term edit form) - single-hyphen indentation & "- None -" option (taxonomy term selector in node edit form)
IMHO we should stick with one style, and it is pretty obvious that the first style would be best for all uses: more indentation makes the hierarchy easier to grok at first sight and the hyphens around none may be confused with the indentation, while brackets are cristal clear.
If you agree, I will provide a patch on monday or so.
Comment | File | Size | Author |
---|---|---|---|
#20 | inconsistent_taxonomy_selects.patch | 4.73 KB | drawk |
#15 | inconsistent_taxonomy_selects-15.patch | 5.58 KB | lilou |
#8 | inconsistent_taxonomy_selects-8.patch | 5.17 KB | Pancho |
#4 | inconsistent_taxonomy_selects-4.patch | 5.18 KB | Pancho |
#2 | inconsistent_taxonomy_selects.patch | 1.64 KB | vladimir.dolgopolov |
Comments
Comment #1
PanchoOh, the HTML filter stripped off the "‹none›" options in 1 and 2, nevermind...
Comment #2
vladimir.dolgopolov CreditAttribution: vladimir.dolgopolov commentedI agree, a nice suggestion, but where is the patch?
So I made the patch.
I think replacing "- None -" with "< none >" will leads to some translate work, but nevertheless.
I found "Contact" and "Forum" also have the hyphens around: '- Please choose -'.
What to do with them?
Comment #3
vladimir.dolgopolov CreditAttribution: vladimir.dolgopolov commentedComment #4
PanchoDidn't come up with a patch, because I've been waiting for some input first.
In your patch, the spaces inside the brackets don't comply with what is used throughout Drupal. Changed this.
Replacing t('- None -') with '<'. t('none') .'>' doesn't lead to translation work, as the string 'none' is already used in core. We're just removing the old string.
In the contrary, in Contact and Forum we will have a true string change, as I corrected t('- Please choose -') to '<'. t('Please choose') .'>', where the string 'Please choose' is not yet used in core. I still think we should do this.
Also added the brackets in Poll and did some minor code-style improvements.
Comment #5
keith.smith CreditAttribution: keith.smith commentedI can't do a full review right now, but as an FYI, there was some previous discussion about what to call the default options in the comment at http://drupal.org/node/180109#comment-610437.
Comment #6
PanchoTook a look at that issue, but the hyphens suggested there are a bad idea, as we are also using hyphens for indentation. That's confusing and looks weird.
Given that webchick erred, when she saw the hyphenated solution as an informal standard, I think we should go with this for now and make this themeable in D7.
Comment #7
webernet CreditAttribution: webernet commentedCode style -- 'dots'/periods should touch quotes.
Further details at http://drupal.org/coding-standards and http://drupal.org/project/coder
Comment #8
PanchoCorrected codestyle as requested in #7.
Comment #9
vladimir.dolgopolov CreditAttribution: vladimir.dolgopolov commentedThe patch works well.
About themeable it in D7 - do you propose to mark "a special options" via CSS?
E.g. option "< none >" will have css class 'color: gray;'. Is this your point?
Comment #10
cburschkaSelectively disabled/themable options - for checkboxes, radios and selects - are definitely a very needed feature for D7. Of course, in this case, we don't want to actually disable the option, because the idea of the "none" option in the first place is to remind the user of selecting something before submitting.
Comment #11
PanchoThe next tester may set this to rtbc.
Comment #12
vladimir.dolgopolov CreditAttribution: vladimir.dolgopolov commentedThere have passed 12 days. Mark this rtbc.
Comment #13
PanchoNeeds to be fixed in D7/HEAD first, then backported to D6.
Comment #14
Pancho#8 is still RTBC for D6, but first we want this to be themeable in D7 (see #6). Therefore back to CNW.
Comment #15
lilou CreditAttribution: lilou commentedReroll against D7.
Comment #18
lilou CreditAttribution: lilou commentedIgnore DrupalTestbedBot ;-)
Comment #19
Sutharsan CreditAttribution: Sutharsan commentedMoving all usability issues to Drupal - component usability.
Comment #20
drawk CreditAttribution: drawk commentedThe patch doesn't cleanly apply because it attempts to patch default.settings.php. I don't see this discussed above, and there are no default.settings.php changes in the previous rolls of this patch, so I'm guessing it was mistake.
Re-roll attached.
Comment #21
Bojhan CreditAttribution: Bojhan commentedPlease provide image to easily show diffrences.
Comment #22
yoroy CreditAttribution: yoroy commentedBump with another request for before/after screenshots
Comment #23
catchComment #24
Anonymous (not verified) CreditAttribution: Anonymous commentedThe last submitted patch failed testing.
Comment #25
xjmSee also:
#12089: Better method of showing forum containers in form
#284917: Allow FAPI select, radios, and checkboxes to specify some options as disabled
This line in taxonomy_form_term is the crux of the matter, to my mind:
Definitely not separation of content from presentation. Plus, in a screen reader, this becomes (e.g.) "parent term", "dash child term"... "dash dash dash dash dash subterm." There's definitely a lot that could be done to improve usability of these form elements. A number of contrib widgets that try to address this usability issue are available in D6 with Content Taxonomy.
It seems like it would be better to move all nesting/indentation of taxonomy term names into the theming layer--and to be able to re-theme form options generally without having to override the entire form element.
Comment #26
xjmI don't think I changed the component... apparently usability doesn't exist as a component anymore?
I guess markup fits, sorta.
Comment #27
yoroy CreditAttribution: yoroy commentedWorth doing, this.
Comment #28
pingers CreditAttribution: pingers commentedThis looks done in 8.x.
Comment #29.0
(not verified) CreditAttribution: commentedadded code tags -sepeck