Not sure if this goes under theme system or other.

When fields end in a '?', the theme_form_element function shows a label like this "Label?:" which looks weird. I added a patch to check if it ends in a question mark, and if so, don't add the colon to the label.

I know this function is themeable so I could've done it there, but I feel this should be the default behavior for all those fields like "Are you a smoker?", "Add to front page?", etc.

CommentFileSizeAuthor
patch_no_colon_after_question_mark.patch710 bytesRobRoy

Comments

Steven’s picture

This patch isn't very flexible as it only deals with question marks, and ASCII ones at that. A better solution would be one that checks for any punctuation at the end (character class Po), but then we'd get people complaining about their PCRE not supporting Unicode again.

Still, I can't think of any situation inside Drupal where we have a question mark at the end of a label. It is not something I'd advise in any case. Instead of "Are you a smoker?" we tend to have a checkbox "I am a smoker.".

As it is a themable function, I'm leaning towards not committing it.

About the actual code, I think substr($label, -1) would be a more readable version of selecting the last byte.

drumm’s picture

I think any change like this should come with guidelines for usage. In English we can have form labels end in ':', '?', or nothing. When is the use of each appropriate? I think the current ':'-only code is good in that it provides consistency. I suspect that any label ending in '?' can be effectively rephrased. I'm not much of an expert on interface element labeling, but I think these issues should be considered.

And this change would need to be implemented on more form elements than text fields.

RobRoy’s picture

Yeah, it's not such a big deal. I guess as my case is pretty unique, using a themed function would be the way to go. Thanks anyways.

chx’s picture

On a side: yesterday hell froze over, Apple went to Intel and what's important to us, Debian sarge became stable. So, no more whining about PCRE -- now can tell everyone (rightly) that anything before 4.3.3 is ancient, and even Debian sports 4.3.10 so...

drumm’s picture

Fixing the issue status since we created a race condition there.