Skip to content

Conversation

@urkle
Copy link

@urkle urkle commented Oct 19, 2010

I also include a test file for the change as well. Commit also applies cleanly to the 1.8.5 tag.

… and radio. Fixed #6063 - Button doesn't support radio input inside label.
@scottgonzalez
Copy link
Member

Thanks for this patch, but there are some changes that need to be made in order for this to land in master. Please review the coding standards and make sure all of your code conforms to these standards. Generated IDs should be in the form of ui-button-{auto-incrementing-number} (see tabs or menu for an example). You're declaring variables multiple times and duplicating logic; please DRY up things like detecting/generating IDs. Use property names such as generatedId instead of buttonHasFakeId. Finally, please add a unit test instead of a visual test.

@urkle
Copy link
Author

urkle commented Jul 21, 2011

Thanks for the pointers.. I've been busy as of late but am freed up on time now so I will look at tweaking this patch according to your recommendations.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants