-
Notifications
You must be signed in to change notification settings - Fork 43
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
base: master
Are you sure you want to change the base?
Conversation
.idea/dictionaries/billsu.xml
Outdated
@@ -0,0 +1,3 @@ | |||
<component name="ProjectDictionaryState"> |
There was a problem hiding this comment.
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"?> |
There was a problem hiding this comment.
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) |
There was a problem hiding this comment.
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) |
There was a problem hiding this comment.
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 )
@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. |
Anthony,
Thanks for your review. Sorry for the absence recently, I was waiting for
the previous pull to settle before continuing to the next task and the
entire thing went over my head as more work started to pickup for my
company.
I will settle the pull as soon as possible and continue on to see what
tasks I can do.
Once again, sorry for the absence.
Best,
Bill Su
…On Sat, Jun 10, 2017 at 10:56 PM, Anthony Crumley ***@***.***> wrote:
@su20yu1919 <https://github.com/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.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#461 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ADkV7Qn6elfrFmYgThhnqiDhisEmHY2-ks5sCq6wgaJpZM4L0XfI>
.
|
Anthony, I have made changes as requested, please let me know what I need to do next to complete this pull request. THanks! |
@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! |
Added Indicator To Determine Whether Member has a children at Birmingham School