Added Indicator To Determine Whether Member has a children at Birmingham School#461
Added Indicator To Determine Whether Member has a children at Birmingham School#461su20yu1919 wants to merge 4 commits intoedbirmingham:masterfrom
Conversation
.idea/dictionaries/billsu.xml
Outdated
| @@ -0,0 +1,3 @@ | |||
| <component name="ProjectDictionaryState"> | |||
There was a problem hiding this comment.
@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.
@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.
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) |
There was a problem hiding this comment.
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! |
4 participants
Added Indicator To Determine Whether Member has a children at Birmingham School