New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: make checkbox selectable via adjacent text #3090
base: master
Are you sure you want to change the base?
Conversation
Thats not how u do it tho, see MDN on how to use for attribute to do what u want. |
This PR is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days. |
Very well, @kloon15. I've adjusted the PR to use id and for attributes instead of wrapping inputs with labels. technically the way I did it before, is one of the ways to do it ; If you rely on MDN guidelines as the fallback or norm, there should be no harm in stating that in the read-before-you-create-a-PR thing that is already up on this repo. |
Clicking the text to select the checkbox the text describes has been an accessibility and a usability UI feature in anything that's worth using for the last 25 years. If you two can't recognize this as maintainers / authors of a frontend app, then there is nothing I can do to help you out... 1st one said "move these elements around" because "some guide says so". ok sure, whatever. Guys... learn to say "I want it like that" instead of "i don't like this"
I can make it appear as it did before the requested change by @kloon15, with the text and the checkbox on one line. If you have some opinion on how that should look in CSS, feel free to share your opinion on that.
"we should" is not a firm imperative statement. If you mean this must be done the same way with all the other checkbox inputs that appear throughout, then say as much: "Please change all the other checkbox inputs as well as these ones, if you want this PR to be accepted."
If you guys don't see the value in this and will continue to "fight" me on this, and you don't understand that this is an important usability feature (i.e. all our users are gods at FPS games and aiming at tiny checkboxes with a mouse is a fun activity for our users), then who am I to interfere? So are we on the same page? I can change all the other checkboxes if that's what it takes. |
@tox2ik I see ur name reflects ur personality; we dont need ur hostility in here, u can take that attitude and make ur own project. |
891e62a
to
34ba11f
Compare
Each checkbox for all settings should now have a label. The label appears under the input in html and should be rendered on the same line as the input. These elements are associated via the id and for attributes, as per MDN strong recommendation. Based on what I could find, there are no other checkboxes in the project. Therefore, this PR no longer brings an inconsistency to the project. I look forward to any further feedback and requests for adjustments to this PR. For your convenience, here is a screenshot of how it looks in Edge, Firefox and Opera. Thank you for the guidance and the feedback, please consider merging this UI/UX improvement. |
Good job! |
The the checkbox is not that easy to hit with a mouse. It's much easier to hit the whole description. HTML lets you do that, so why not use it?
The appearance of the font changed slightly but not significantly after the text was moved into a label because other css rules are affecting it now. I don't think it needs adjustment, but I'm not a UI guy or it looks good enough to me before and after.
The app builds.
Before and after:
The texts appears a little bit smaller when wrapped in a label and the collor is gray instead of black.