ui: Implements the ember-page-title addon to the UI #7118
Conversation
There was a problem hiding this comment.
This looks great @kaxcode! My one request is to add test coverage.
Consul uses Gherkin for acceptance testing, and I suspect since these would be the first title tests, you'll have to add new functionality to the existing steps the Gherkin tests use.
@johncowen will know what to do 🙂
kaxcode
force-pushed
the
feature/ui-implement-ember-page-title
branch
from
95a29f2 to
0976206
Compare
* Installs ember-page-title 5.x * Adds a page title to all template files that need one * Adds an assertion step to test the page titles
kaxcode
force-pushed
the
feature/ui-implement-ember-page-title
branch
from
0976206 to
60b0db8
Compare
There was a problem hiding this comment.
Hey @kaxcode
I gave this a spin and it looks great! Super stoked you got into the testing and down to writing your own steps 🎉
I'd say this is good to merge as is, but I left one comment about looking at that head.hbs file, maybe just check see if thats still needed or not.
| @@ -0,0 +1 @@ | |||
| <title>{{model.title}}</title> | |||
There was a problem hiding this comment.
Was this file only needed for when you are using FastBoot? Maybe try and remove it and see if everything is fine. We don't run in FastBoot so if we don't need this file we could leave it out of the PR.
There was a problem hiding this comment.
I commented this file out and the page titles broke. This is a required file with or without FastBoot.
Installs ember-page-title 5.x
Adds a page title to all template files that need one
Adds an assertion step to test the page titles