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.
In the Node edit sidebar, we shouldn't be using a label where there should just be markup. Labels need to be associated with form elements. This should be a heading styled to look like the rest of the labels in order to be semantically correct:
<label for="edit-changed--2">Last saved</label>
Not saved yet
</div>
<div id="edit-author--2" class="author container-inline form-item form-type-item form-item-author">
<label for="edit-author--2">Author</label>
root
</div>
should be:
<h4 class="label">Last saved</h4>
Not saved yet
</div>
<div id="edit-author--2" class="author container-inline form-item form-type-item form-item-author">
<h4 class="label">Author</h4>
root
</div>
Comment | File | Size | Author |
---|---|---|---|
#33 | 2249089-label-author-last-saved-33.patch | 1.99 KB | mgifford |
#30 | Screen Shot 2014-06-06 at 22.56.26.png | 28.24 KB | lauriii |
#30 | Screen Shot 2014-06-06 at 22.56.14.png | 27.64 KB | lauriii |
#29 | 2249089-label-author-last-saved-29.patch | 1.99 KB | mgifford |
#27 | 2249089-label-author-last-saved-27.patch | 1.99 KB | chaquea |
Comments
Comment #1
mgiffordComment #2
mgiffordComment #3
mgiffordComment #4
mgiffordOk.. I think this works.
EDIT: 2 Little CSS Notes:
I don't think there was any way to do this:
Also, this should apply to things styled like labels too.
Comment #5
mgiffordComment #6
mgiffordHere's a view with the patch.
Comment #7
LewisNymanJust one thing, can we remove the h4? We shouldn't need it.
Comment #8
mgiffordThere is a precedent for h4.label, should we just have a generic label CSS class?
It should be at least
<strong>
which has some semantic meaning.Comment #9
lokapujyaAre "Last Saved" and "Author" just read-only form items with a label?
Comment #10
mgiffordYup. Jut read only. No need for a label as there is no associated form element. Just markup and styling.
Comment #11
lokapujyaMaybe "Last Saved" and "Author" are form elements?
Comment #12
lokapujyaI mean, it is still #type=item.
Comment #13
LewisNymanYeah, let's create a reusable .label class for any element.
Comment #14
mgifford@lokapujya I'm going to have to open up a new issue about the Forms API I think:
https://api.drupal.org/api/drupal/developer!topics!forms_api_reference.h...
This example will produce a Label without an a related text input. It's a bad example. Whether we need #type=item or not I don't know.
We'd need to insert some line breaks though if we were to just remove the item line. It works, but produces:
<h4 class="label inline">Last saved</h4> 04/28/2014 - 13:32<h4 class="label inline">Author</h4> root<div id="edit-revision-information" class="node-form-revision-information form-wrapper"><div class="form-item form-type-checkbox form-item-revision">
@LewisNyman would that just be changing h4.label to:
That would eliminate the need for the .label.inline too.
Comment #15
lokapujyaI noticed the items such as "Menu Settings" are not blue in Chrome. Maybe we should open another issue for that. I was going to fix it with CSS, but it actually seems like the markup is different. Not sure how using Chrome changes the markup.
Comment #16
lokapujya@14 What I am asking is that "Last Saved" and "Author" are actually form elements (just displayed as read only here) and so: would label actually be semantically correct?
Comment #17
mgiffordRegarding the menu settings in Chrome, ya would make sense to make a new issue for that.
It would be fine if we were using the readonly attribute
<input type="text" readonly="readonly" value="myValue">
but that doesn't seem to be getting into Core any time soon #500868: Forms API is missing the readonly form field option.Otherwise it shouldn't have a label.
Comment #18
LewisNymanComment #19
LewisNyman@mgifford Sorry I never got back to you, yep. It would be great if we could kill two birds with one stone.
Comment #20
mgiffordHow's this as a generalized .label version of this patch?
Comment #22
mgifford20: 2249089-label-author-last-saved-20.patch queued for re-testing.
Comment #24
LewisNyman20: 2249089-label-author-last-saved-20.patch queued for re-testing.
Comment #25
crasx CreditAttribution: crasx commented@mgifford that looks good to me!
however the h4's should float left and maybe have some right padding to match current designs
also, is this the correct spot for a header? should it be strong instead?
Comment #26
chaquea CreditAttribution: chaquea commentedLooks good, will just make it display inline so it looks the same as the previous label appearance.
Comment #27
chaquea CreditAttribution: chaquea commentedAdded the inline style, attaching an image of the result.
Comment #28
LewisNymanThanks. I found a minor problem related to coding standards:
This property is incorrectly indented, it should be two spaces as per our coding standards.
Comment #29
mgiffordI removed the tab...
Comment #30
lauriiiTested this manually and this seems to work.
Here's pictures of this:
Before:
After:
Comment #32
lauriiiComment #33
mgiffordthis should be able to be set back to rtbc when green. It's just a reroll. I think for spacing.
Comment #34
lauriiiTested this and seems to work as expected.
Comment #35
webchickHey, folks. I'm really sorry about the delay on this. My post-DrupalCon life got a bit hectic. :(
Looks like this issue is specific to Seven and the manipulations it does to the node form, so this fix should be sufficient.
Committed and pushed to 8.x. Thanks!
Comment #37
mgiffordThanks @webchick. Life sometimes does that. Happy to have you back.