lil-gui icon indicating copy to clipboard operation
lil-gui copied to clipboard

Enable / disable a checkbox when clicking on the label

Open jellehak opened this issue 2 years ago • 4 comments

This does two things:

  • Enable / disable a checkbox when clicking on the label
  • Focus the text input when clicked on the label

jellehak avatar Mar 13 '22 08:03 jellehak

Hey thanks for writing this—you just want the checkbox to toggle when clicking the controller name, right?

At one point I actually had it the way you do right now, where the container for every controller was <label>, but it broke things (can't remember exactly what).

georgealways avatar Mar 16 '22 22:03 georgealways

@georgealways Yes you are right. ( it also works for other input fields, as it will put focus on the element when clicking on the label. )

A possible "threat" could be when a user has custom styling applied to a label element. But that is then probably intended.

jellehak avatar Mar 18 '22 08:03 jellehak

Hi @jellehak, if you haven't already, would you mind ticking the "Allow edits from maintainers" checkbox for this PR? ty!

georgealways avatar May 31 '22 19:05 georgealways

Thanks for putting this together, just had a chance to try it out.

So it's nice that this works on some of the other controllers too, but changing the container of every controller to <label> comes with a few drawbacks:

  • The <select> in OptionController winds up in a sort of broken state when clicking on the name (receives two click events).
  • It feels inconsistent that hovering over names triggers hover effects on some controllers and not others (option and color).
  • It feels ambiguous for controllers that can have two inputs (number and color)—which should be focused on click?

There is one benefit to this approach though: Changing the container element to a <label> would allow us to get rid of all the manual aria-labelledby attributes, and assistive tech would still work the same.

But, if the goal here is just to toggle checkboxes via clicking the label, then I'd be slightly more comfortable with something like this in BooleanController's constructor:

this.$name.addEventListener( 'click', () => this.$input.click() );

If that happens, we should also modify the CSS so the entire element has the same hover behavior and cursor.

georgealways avatar Jun 19 '22 12:06 georgealways

Hi there, revisiting old PR's today—if the intent is to make checkbox labels clickable, then I think changing the container element of every controller in the library is too drastic a change.

If you'd like to try an approach like the one I described above, edit this PR and I'll re-open it. Just be sure to remove the changes that aren't relevant to your PR (onchange, pnpm lockfile).

Thanks, -g

georgealways avatar Mar 11 '23 14:03 georgealways