-
Notifications
You must be signed in to change notification settings - Fork 61
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
Add text for no results #94
Conversation
Codecov Report
@@ Coverage Diff @@
## master #94 +/- ##
=======================================
Coverage 74.24% 74.24%
=======================================
Files 84 84
Lines 730 730
=======================================
Hits 542 542
Misses 188 188 Continue to review full report at Codecov.
|
Not 100% sure but think this may show even if there has been no search. |
0b639f5
to
9f7c4ea
Compare
Updated :) |
@kategengler bump |
@@ -58,6 +58,7 @@ | |||
"ember-router-scroll": "~0.3.0", | |||
"ember-sanitize": "^2.0.2", | |||
"ember-source": "~2.14.1", | |||
"ember-truth-helpers": "^1.3.0", |
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 am not a fan of truth-helpers. I prefer a local and
helper so as not to make the rest of the helpers from truth-helpers available.
@@ -68,5 +68,7 @@ | |||
{{/search-result-set}} | |||
{{else if search.isRunning}} | |||
{{dot-spinner}} | |||
{{else if (and search.performCount query.length)}} | |||
<h4 class="result-info test-result-info">No results for "{{query}}"</h4> |
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.
The test-
classes are meant to be unique, at least per route. test-result-info
shouldn't be repeated here.
@@ -68,5 +68,7 @@ | |||
{{/search-result-set}} | |||
{{else if search.isRunning}} | |||
{{dot-spinner}} | |||
{{else if (and search.performCount query.length)}} |
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 think this will erroneously show when a search has happened and a query has been typed, but it is not a valid query that triggers a search. Take a look at queryIsValid
CP in app/components/large-search.js
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.
Please add test(s)
Fixes #64