Skip to content
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

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

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

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.

@@ -14,6 +16,7 @@ $(document).on 'ready page:load', ->
$('#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