Look around in Drupal6 and you'll find three different ways hierarchical lists are rendered in selectboxes:

  1. double-hyphen+space indentation & "<None>" option (menu parent item in node edit form)
  2. single-hyphen indentation & "<none>" option (menu parent item & related terms in term edit form)
  3. 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.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Pancho’s picture

Oh, the HTML filter stripped off the "‹none›" options in 1 and 2, nevermind...

vladimir.dolgopolov’s picture

I 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?

vladimir.dolgopolov’s picture

Status: Active » Needs review
Pancho’s picture

Component: taxonomy.module » other
FileSize
5.18 KB

Didn'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.

keith.smith’s picture

I 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.

Pancho’s picture

Took 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.

webernet’s picture

Status: Needs review » Needs work

Code style -- 'dots'/periods should touch quotes.

Further details at http://drupal.org/coding-standards and http://drupal.org/project/coder

Pancho’s picture

Status: Needs work » Needs review
FileSize
5.17 KB

Corrected codestyle as requested in #7.

vladimir.dolgopolov’s picture

The 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?

cburschka’s picture

Selectively 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.

Pancho’s picture

The next tester may set this to rtbc.

vladimir.dolgopolov’s picture

Status: Needs review » Reviewed & tested by the community

There have passed 12 days. Mark this rtbc.

Pancho’s picture

Version: 6.x-dev » 7.x-dev

Needs to be fixed in D7/HEAD first, then backported to D6.

Pancho’s picture

Assigned: Unassigned » Pancho
Status: Reviewed & tested by the community » Needs work

#8 is still RTBC for D6, but first we want this to be themeable in D7 (see #6). Therefore back to CNW.

lilou’s picture

Status: Needs work » Needs review
FileSize
5.58 KB

Reroll against D7.

Patch failed to apply. More information can be found at http://testing.drupal.org/node/13885. If you need help with creating patches please look at http://drupal.org/patch/create

Patch failed to apply. More information can be found at http://testing.drupal.org/node/13884. If you need help with creating patches please look at http://drupal.org/patch/create

lilou’s picture

Ignore DrupalTestbedBot ;-)

Sutharsan’s picture

Component: other » usability

Moving all usability issues to Drupal - component usability.

drawk’s picture

The 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.

Bojhan’s picture

Please provide image to easily show diffrences.

yoroy’s picture

Bump with another request for before/after screenshots

catch’s picture

Title: Usability: Inconsistent Taxonomy selects » Usability: Unify select option formatting
Anonymous’s picture

Status: Needs review » Needs work

The last submitted patch failed testing.

xjm’s picture

Component: markup » ajax system
Assigned: Unassigned » Pancho

See 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:

$options[$item->tid] = str_repeat('-', $item->depth) . $item->name;

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.

xjm’s picture

Version: 7.x-dev » 8.x-dev
Component: usability » markup
Assigned: Pancho » Unassigned

I don't think I changed the component... apparently usability doesn't exist as a component anymore?

I guess markup fits, sorta.

yoroy’s picture

Component: ajax system » markup
Assigned: Pancho » Unassigned
Issue tags: +Usability, +Novice, +consistency

Worth doing, this.

pingers’s picture

Status: Needs work » Fixed

This looks done in 8.x.

Status: Fixed » Closed (fixed)
Issue tags: -Usability, -Novice, -consistency

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

Anonymous’s picture

Issue summary: View changes

added code tags -sepeck