-
Notifications
You must be signed in to change notification settings - Fork 4
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
Super basic form app/'context' trail #92
Super basic form app/'context' trail #92
Conversation
&:hover { | ||
color: color(blue); | ||
} | ||
} |
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.
just cripped all of this from the patient scss
}, | ||
templateContext() { | ||
return { | ||
src: `${ window.location.origin }/formapp/${ this.model.id }`, |
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.
? stabs in the dark here
test/integration/forms/form.js
Outdated
.find('[data-patients-region] .app-nav__link') | ||
.last() | ||
.click({ force: true }); | ||
.wait(2000); |
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.
waiting doesn't feel great, but not sure the best way to make sure the form is loaded? Should it be stubbed out with form data? Or somesuch?
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.
I don't think we should test the contents of the iframe, just the src for now. but https://github.com/RoundingWell/RWell/blob/develop/cypress/support/commands.js#L42 when we do.
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.
I don't think we need the wait to check the src
?
Codecov Report
@@ Coverage Diff @@
## develop #92 +/- ##
===========================================
- Coverage 97.63% 97.57% -0.06%
===========================================
Files 79 82 +3
Lines 1436 1442 +6
===========================================
+ Hits 1402 1407 +5
- Misses 34 35 +1
Continue to review full report at Codecov.
|
Test summaryRun details
View run in Cypress Dashboard ➡️ This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard |
src/js/apps/forms/form/form_app.js
Outdated
import { LayoutView } from 'js/views/forms/form/form_views'; | ||
|
||
export default App.extend({ | ||
currentAppOptions() { |
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 is the default. you can remove this
</a> | ||
</div> | ||
<h1>{{ name }}</h1> | ||
<iframe src="{{ src }}" class="form__frame"></iframe> |
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.
does src="/formapp/{{ id }}"
work?
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.
form loads, so yeah I think so!
test/integration/forms/form.js
Outdated
@@ -1,21 +1,30 @@ | |||
context('Patient Form', function() { | |||
specify('routing to form', function() { | |||
cy |
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.
is this visit needed? If so that may be a problem
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.
nope, sure isn't!
test/integration/forms/form.js
Outdated
.find('[data-patients-region] .app-nav__link') | ||
.last() | ||
.click({ force: true }); | ||
.wait(2000); |
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.
I don't think we should test the contents of the iframe, just the src for now. but https://github.com/RoundingWell/RWell/blob/develop/cypress/support/commands.js#L42 when we do.
509b06c
to
c678cac
Compare
test/integration/forms/form.js
Outdated
.get('[data-nav-region]') | ||
.should('be.visible'); | ||
.get('iframe') | ||
.should($iframe => { |
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.
its('src').should('contain'
I think
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.
gave that a shot, but was getting a 'could not find property' of src
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.
hmm maybe its doesn't work on attributes.. how about should('have.attr'
?
test/integration/forms/form.js
Outdated
.find('[data-patients-region] .app-nav__link') | ||
.last() | ||
.click({ force: true }); | ||
.wait(2000); |
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.
I don't think we need the wait to check the src
?
c678cac
to
148bbaf
Compare
</a> | ||
</div> | ||
<h1>{{ name }}</h1> | ||
<iframe height="100px" src="/formapp/{{ id }}" class="form__frame"></iframe> |
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.
I'm just now noticing the height here. all styling should be in the class.
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.
ah crap. I put this in to test its
to see if it would wok on props like this and forgot to remove
test/integration/forms/form.js
Outdated
.get('[data-nav-region]') | ||
.should('be.visible'); | ||
.get('iframe') | ||
.should($iframe => { |
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.
hmm maybe its doesn't work on attributes.. how about should('have.attr'
?
148bbaf
to
e4e1af1
Compare
not sure about...most of this. So rip 'er up and lemme know what you think