Problem/Motivation

Label with a for attribute and are display block have an invisible hit area that get is accidentally clicked.
This is especially problematic with the image field as it will launch the browse window.

Proposed resolution

Provide a way to keep the hit area of the label to the text with at the same time letting it clear the line. Note: :after tags won't work well with the required markers now in CSS in the :after

Also to indicate the label is clickable use cursor: pointer.

Remaining tasks

User interface changes

The hit area of labels.

API changes

CommentFileSizeAuthor
#13 2277661-label-block-width-13.patch855 bytesalexrayu
#9 Configure_site___Drupal.png555.16 KBjoelpittet
#9 Create_Article___s9374746d1791339_s2_simplytest_me.png125.1 KBjoelpittet
#9 Configure_site___Drupal 2.png474.72 KBjoelpittet
#9 Create_Article___s8d1185ab31415ca_s2_simplytest_me.png110.65 KBjoelpittet
#4 interdiff.txt684 bytesalexrayu
#4 2277661-label-block-width-4.patch869 bytesalexrayu
#1 2277661-label-block-width-1.patch855 bytesjoelpittet

Comments

joelpittet’s picture

joelpittet
he/him
Primary language English
Location Vancouver
commented
Status: Active » Needs review
Issue tags: +Needs manual testing
StatusFileSize
new 2277661-label-block-width-1.patch855 bytes

@lewisnyman and I tried a few different approaches yet this one was the cleanest we came up with so far. Any other suggestions are welcome, though this one does look to work quite well so give it a try.

lewisnyman’s picture

lewisnyman
he/him
commented 
+++ b/core/themes/seven/style.css
@@ -761,11 +761,14 @@ fieldset {
+label[for] {

I remember having this discussion for html5 boilerplate years ago: https://github.com/h5bp/html5-boilerplate/pull/418

Anonymous’s picture

Anonymous (not verified) commented

Ooh, well if Paul Irish spurned it, I don't know... :-)

I'm wary of display: table, but it does seem to work! The width is now calculated to the size.

alexrayu’s picture

alexrayu commented
StatusFileSize
new 2277661-label-block-width-4.patch869 bytes
new interdiff.txt684 bytes

Please consider using inline-block instead of table.

joelpittet’s picture

joelpittet
he/him
Primary language English
Location Vancouver
commented

@alexrayu please test both and let me know which one does what is intended.

@JamesLafrère ha nice find. We do know that our form elements attach their labels with for attributes. And the cursor helps illustrate the problem. I believe there is also now a type=> label that doesn't have to tie to an element nor does checkboxes and radios so that's why I tied it to the for attribute selector.

alexrayu’s picture

alexrayu commented

@joelpittet I looked at both, and they both seem to produce same result in practice. In theory, however, having label an inline-block seems to be semantically correct. I tested these on node form.

lewisnyman’s picture

lewisnyman
he/him
commented

If we can get away with display: inline-block with no repercussions let's do it. display: table feels dirty.

Anonymous’s picture

Anonymous (not verified) commented
Status: Needs review » Reviewed & tested by the community

Yep, inline-block works just the same, tested on Chrome/FF/Safari. Marking as RTBC.

joelpittet’s picture

joelpittet
he/him
Primary language English
Location Vancouver
commented
Status: Reviewed & tested by the community » Needs work
StatusFileSize
new Create_Article___s8d1185ab31415ca_s2_simplytest_me.png110.65 KB
new Configure_site___Drupal 2.png474.72 KB
new Create_Article___s9374746d1791339_s2_simplytest_me.png125.1 KB
new Configure_site___Drupal.png555.16 KB

@JamesLefrère and @alexrayu I'm not sure the testing was very thorough as I did originally try with display: inline-block as the first thing I did, and got creative with :after trickery as well but that conflicted with required markers.

So here's a bit of proof in pictures.



Anonymous’s picture

Anonymous (not verified) commented
Status: Needs work » Needs review

Whoopsy-daisy. I didn't actually notice the inline-block issues! I think this is going to have to be display: table. It's not such a big deal though eh? (Patch 1 to review?)

Anonymous’s picture

Anonymous (not verified) commented
Title: Block Label's with a for attribute have an invisible hit area that get is accidently clicked » Block labels with a for attribute have an invisible hit area that is accidently clicked
alexrayu’s picture

alexrayu commented

Yes. Sadly inline-block won't do then.

alexrayu’s picture

alexrayu commented
StatusFileSize
new 2277661-label-block-width-13.patch855 bytes

Reuploading the joels patch to have it latest.

Anonymous’s picture

Anonymous (not verified) commented
Status: Needs review » Reviewed & tested by the community
joelpittet’s picture

joelpittet
he/him
Primary language English
Location Vancouver
commented
Assigned: Unassigned » yesct

Here:D

yesct’s picture

yesct commented
Assigned: yesct » Unassigned

:)

joelpittet’s picture

joelpittet
he/him
Primary language English
Location Vancouver
commented
Title: Block labels with a for attribute have an invisible hit area that is accidently clicked » Labels with display:block and 'for' attribute have an invisible hit area that is accidently clicked

Betterish title

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Status: Reviewed & tested by the community » Fixed

Really sorry about how long this sat here. My post-DrupalCon life got a bit hectic. Confirmed the fix only makes the image browse appear when I click directly on the text, not just somewhere nearby.

Committed and pushed to 8.x. Thanks!

  • webchick committed 92bcdc5 on 8.x 
    Issue #2277661 by alexrayu, joelpittet: Fixed Labels with display:block...

Status: Fixed » Closed (fixed)

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

brankoc’s picture

brankoc commented
Related issues: +#3020907: Avoid horizontal scrollbar in media library dialog when using small viewports

+related issue