This is an inappropriate use of the label element in html.
#markup' => '<label>' . t('Did you mean') . '</label>' . l($new_keywords, $query->get_path($new_keywords)),
Should probably be:
#markup' => '<div class="label">' . t('Did you mean') . l($new_keywords, $query->get_path($new_keywords)) . '</div>',
For the moment it would also need to be styled with:
div.label {
float: left; /* LTR */
font-weight: normal;
width: 10em;
}
if you want to have it look like a label. However, really this should just be emphasized text, so this would work well too:
#markup' => '<p><strong>' . t('Did you mean') . l($new_keywords, $query->get_path($new_keywords)) . '</strong></p>',
| Comment | File | Size | Author |
|---|---|---|---|
| #17 | 1098038-no-label-strong-17-D6.patch | 1.77 KB | pwolanin |
| #17 | suggestions-label.png | 78.89 KB | pwolanin |
| #17 | suggestion-patched-dd.png | 71.59 KB | pwolanin |
| #9 | 1098038-no-label-em-9.patch | 1.76 KB | pwolanin |
| #9 | 1098038-no-label-strong-9.patch | 1.77 KB | pwolanin |
Comments
Comment #1
pwolanin commentedSo label should only be used inside a form?
Comment #2
mgiffordIt needs to be tied to a form element as per - http://www.w3schools.com/tags/tag_label.asp
Labels are always tied to a specific form element and it isn't good to use them for generic markup.
Comment #3
pwolanin commentedhow about a dl?
Comment #4
pwolanin commentedor better yet, a theme function?
Comment #5
mgiffordI like the idea behind #4, but after applying it I didn't get the 'Did you mean' HTML being spit out to the screen.
I haven't had a chance to trace this down, but the themable function didn't show up.
Comment #6
jpmckinney commentedYou probably need to clear your theme cache. Drupal won't pick up the new theme functions otherwise.
Comment #7
mgiffordI'd thought I'd done that, but it's working now, so perhaps not. Thanks.
This is a good patch that adds more flexibility in how this suggestion is displayed. Always useful
Comment #8
jpmckinney commentedPatch looks good. Not sure how dl/dts look with default styling in D7, though.
Comment #9
pwolanin commentedHere are a bunch of screen shots plus patches with EM and STRONG options showing the old styling and various new styling. By default a DL element seems to have a bit of excess margin on the top, but a quick hack to make it look better in Bartik is to add the "form-item" class (not really a lie since it's inside the FORM element).
Existing appearance w/ LABEL: http://drupal.org/files/issues/search-label.png
Appearance per #4 using STRONG: http://drupal.org/files/issues/search-dl-strong.png
Appearance per #4 using EM: http://drupal.org/files/issues/search-dl-em.png
Per #9 with added class using STRONG: http://drupal.org/files/issues/search-dl-form-item-strong.png
Per #9 with added class using EM: http://drupal.org/files/issues/search-dl-form-item-em-results.png
Comment #10
jpmckinney commentedI vote #9 with strong.
Comment #11
pwolanin commentedok, let's go with that for now - it's less visual change from what we have with LABEL.
committed to 7.x - do we want to port to 6.x-2.x or 6.x-1.x?
Comment #12
jpmckinney commentedWill probably get more attention in the 1.x queue.
Comment #13
mgiffordThanks! And agreed that most folks using this will presently be with the D6 issue queue.
Comment #14
pwolanin commentedIs this safe to backport?
Comment #15
mgiffordThere is no reason not to back-port it that I can see. It's not proper HTML, but a legacy approach to formatting that is still in D7 in some places unfortunately.
Comment #16
pwolanin commentedstill need the 6.x patch
Comment #17
pwolanin commentedHere's a D6 patch plus screenshots to compare before and after the patch. The text looks basically the same in Garland, but the alignment changes a bit.
Comment #18
jpmckinney commentedI can live with the alignment change, but maybe others can't stand it.
Comment #19
nick_vhLooking good. Committed, one issue down - many more to go for the 6 version :-)
Comment #20
nick_vh