Enhancement/move modal to inline #982
Conversation
|
@dasManaswini - I took a look at the failing tests and I realized the tests are failing because of the change you made. The reason is that the tests are expecting the modal to appear when the patient is saved. Those lines need to be changed to instead detect the inline message and then the tests should pass. Let me know if you need any help or further instruction on this. |
|
@jkleinsc I have changed those test cases that you had mentioned, still I am getting some errors. Can you please help me debug those errors? |
|
@dasManaswini I'll take a look today and see what the problem is. |
tests/acceptance/patients-test.js
Outdated
| andThen(function() { | ||
| assert.equal(find('.modal-title').text(), 'Patient Saved', 'Patient record has been saved'); | ||
| assert.equal(find('.message').text(), 'Patient Saved', 'Patient record has been saved'); | ||
| assert.equal(find('.modal-body').text().trim(), 'The patient record for John Doe has been saved.', 'Record has been saved'); |
There was a problem hiding this comment.
This test is failing because .modal-body isn't displayed, .message is
tests/acceptance/patients-test.js
Outdated
| andThen(function() { | ||
| assert.equal(find('.modal-title').text(), 'Patient Saved', 'Patient record has been saved'); | ||
| assert.equal(find('.message').text(), 'Patient Saved', 'Patient record has been saved'); |
There was a problem hiding this comment.
the text you are looking to find should be 'The patient record for John Doe has been saved.', not 'Patient Saved'
There was a problem hiding this comment.
the modal close shouldn't be here
| }); | ||
| andThen(function() { | ||
| assert.equal(find('.modal-title').text(), 'Patient Saved', 'Patient record has been saved'); | ||
| assert.equal(find('.message').text(), 'Patient Saved', 'Patient record has been saved'); |
There was a problem hiding this comment.
the text you are looking to find should be 'The patient record for John Doe has been saved.', not 'Patient Saved'
|
@dasManaswini one more thing in looking at the tests. The following code in the tests: waitToAppear('.message');should be changed to: waitToAppear('.message:contains(The patient record for John Doe has been saved)'); |
|
Thanks a lot for your guidance @jkleinsc I have made necessary changes as you had mentioned. |
There was a problem hiding this comment.
@dasManaswini thanks for making the updates. I'm glad to see the tests are now passing. The only thing I see now is that it looks like tests/acceptance/patients-test.js has been reformatted and I'd prefer not to merge it in that way as it makes comparing changes difficult. Does your text editor use tabs instead of spaces?
tests/acceptance/patients-test.js
Outdated
| }); | ||
| }); | ||
| } | ||
| import Ember from 'ember'; |
There was a problem hiding this comment.
Did you reformat this file? It appears as though the whole file has been changed.
There was a problem hiding this comment.
@jkleinsc I had used eslint to fix the errors of the particular file.May be because of that the whole file appears to be reformatted.I am going to replace the existing file with the previous one so that you can easily merge it.
recrsn
force-pushed
the
enhancement/move_modal_to_inline
branch
from
e0bdcfd to
b1002f4
Compare
|
@jkleinsc I have updated the branch with the necessary modifications. Please let me know if any further changes are to be made. |
|
Thanks for sticking with this PR @dasManaswini! I think its ready to go, so I am going to merge it in. |
Fixes #377
Progress in work of #958 and #973
Changes proposed in this pull request:
cc @jkleinsc @jglovier