-
Notifications
You must be signed in to change notification settings - Fork 20
Customized Checkbox -- Associated With The Issue 571 on Webmaker-app #91
Conversation
Hi @droxxr. Thanks for putting this patch together! Since we're just looking to increase touch target size for checkboxes, a cleaner way to do this is to surround the checkboxes with DIVs that have some padding and then use JavaScript to make clicks on those DIVs toggle their child checkbox. Would it be possible for you to refactor your patch to use this approach? |
@gvn Just to clarify things, would this mean that I will not need to style or create a checkbox to replace the default browser checkbox? So, I would just need to keep the default browser checkbox, but do as you said, use JavaScript to make clicks on those DIVS [surrounding the checkbox] toggle their child checkbox. Does this mean that those DIVS are invisible around the default checkboxes? |
Yep, indeed. 👍 Those divs would be invisible and just there to create a larger click target. You can maintain the current browser checkbox style as is... |
Actually, @droxxr , I see that you have a patch open for Webmaker App that creates larger checkboxes. Getting that landed in Can you open a PR here on webmaker-login-ux with the changes you made in mozilla/webmaker-android#621 ? |
@gvn phew.. I'm glad I don't have to go through the process of doing it in JavaScript ;) Sure, I'll make a pull request. Just to clarify things. Right now, I have a pull request for webmaker-app that contains the CSS code for the larger checkbox (currently in webmaker-app -> fixed -> styles -> login.less). So, you want me to move this CSS code over to webmaker-login-ux (specfically webmaker-login-ux -> src -> less -> webmakerLogin.less)? |
@droxxr Yes, it would be ideal if you can move your changes upstream to |
Fixed title case for template
93ff89f
to
987d148
Compare
@gvn I moved all the changes upstream to webmaker-login-ux, in my latest commit (https://github.com/droxxr/webmaker-login-ux/tree/issue%23571-webmaker-app) and also made minor adjustments to it. Please let me know if you would prefer a new pull request instead and of any other problems. Thanks. |
Thanks! This looks great. I did a rebase for you so that we can land your patch (there were some conflicts with the committed built files, which I resolved). I merged via #100 Closing this ticket. |
Updates:
Implementation note:
Fixes: