Skip to content

Conversation

@nqst
Copy link
Contributor

@nqst nqst commented Dec 8, 2025

Enable clicking on toggle switch labels on the Set up a Webhook and Board Settings pages:



@brunoprietog
Copy link
Contributor

There should be only one label per input. To avoid having to generate the ID, you can simply wrap the input inside the label and everything will be clickable. The element for screen readers should also remain inside the label in that case.

@andyra
Copy link
Contributor

andyra commented Dec 8, 2025

Nice improvement, @nqst! After wrapping the control in a label like @brunoprietog suggested, I'd also recommend adding pointer: cursor; to the .switch class so it's a little more clear that the text is clickable when using a mouse.

@nqst nqst marked this pull request as draft December 8, 2025 18:50
<span class="switch__btn round"></span>
<span class="for-screen-reader"><%= item.text %></span>
<li class="list-style-none margin-none">
<label class="toggler__visible-when-off flex align-center gap cursor-pointer">
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
<label class="toggler__visible-when-off flex align-center gap cursor-pointer">
<label class="toggler__visible-when-off flex align-center gap">

I'm not entirely sure about using cursor-pointer on the text labels like this — it feels a bit distracting to me. I'd prefer keeping the default cursor there.

That said, it's up to you. Feel free to apply the suggestion if you agree, or leave it as is if you think the pointer cursor works better.

@nqst
Copy link
Contributor Author

nqst commented Dec 8, 2025

Thanks for the helpful feedback, @brunoprietog @andyra! I've addressed everything.

@nqst nqst marked this pull request as ready for review December 8, 2025 22:57
Copy link
Contributor

@andyra andyra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good 👍

@andyra andyra merged commit bed0e4b into basecamp:main Dec 10, 2025
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants