Skip to content

Conversation

@su20yu1919
Copy link

Added Indicator To Determine Whether Member has a children at Birmingham School

@@ -0,0 +1,3 @@
<component name="ProjectDictionaryState">
Copy link
Contributor

Choose a reason for hiding this comment

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

@su20yu1919 Please remove these ide files from the pull request. Adding the .idea directory to the .gitignore file would keep these from being included.

.idea/vcs.xml Outdated
@@ -0,0 +1,6 @@
<?xml version="1.0" encoding="UTF-8"?>
Copy link
Contributor

Choose a reason for hiding this comment

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

@su20yu1919 Please remove this IDE file as well.


$('#member_ids').on 'select2:select', (e) ->
$('#participation_member_id').val(e.params.data.id)
console.log(e)
Copy link
Contributor

Choose a reason for hiding this comment

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

Debug lines should not be included in the pull request.

$('#phone').text(e.params.data.phone)
$('#identity').text(e.params.data.identity)
$('#email').text(e.params.data.email)
$('#children_in_birmingham_school').text(e.params.data.children_in_birmingham_school)
Copy link
Contributor

Choose a reason for hiding this comment

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

This may not be the way to check the checkbox with jQuery. See https://learn.jquery.com/using-jquery-core/faq/how-do-i-check-uncheck-a-checkbox-input-or-radio-button/

Maybe something like...

$('#children_in_birmingham_school').prop( "checked", e.params.data.children_in_birmingham_school )

@anthonycrumley
Copy link
Contributor

@su20yu1919 I did the review shortly after submission of the pull request but that was the first time I used the Github code review functionality. Unfortunately, I did not realize that there is an extra step to submit the review after all the comments are made. I apologize for the delayed response.

@su20yu1919
Copy link
Author

su20yu1919 commented Jun 11, 2017 via email

@su20yu1919
Copy link
Author

Anthony, I have made changes as requested, please let me know what I need to do next to complete this pull request. THanks!

@rthbound
Copy link
Contributor

@anthonycrumley it looks like your requested changes have been completed. I've gone ahead and resolved the conflicts to catch this branch up with edbirmingham/master if you'd like to give it one more look and sign off!

Thank you for your contribution @su20yu1919!

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants