View tree search filter #720
Conversation
Refs Issue-677
Refs Issue-677
Refs Issue-677
Refs Issue-677
Refs Issue-677
app/controllers/view-tree.js
Outdated
| @@ -12,6 +12,19 @@ export default Controller.extend({ | |||
| components: false | |||
| }, | |||
|
|
|||
| searchText: "", | |||
There was a problem hiding this comment.
Please use single quotes.
We're also trying to document any new code so that we slowly migrate to a more documented code base.
/**
* Bound to the search field to filter the component list.
*
* @property searchText
* @type {String}
* @default ''
*/
app/controllers/view-tree.js
Outdated
| @@ -12,6 +12,19 @@ export default Controller.extend({ | |||
| components: false | |||
| }, | |||
|
|
|||
| searchText: "", | |||
|
|
|||
| filteredList: computed('model', 'searchText', function() { | |||
There was a problem hiding this comment.
computed('model.[]', 'searchText'
/**
* The filtered view list.
*
* @property filteredList
* @type {Array<Object>}
*/
app/controllers/view-tree.js
Outdated
| return this.get('model'); | ||
| } | ||
|
|
||
| let filtered = this.get('model').filter(v => v.value.name.includes(searchText)); |
There was a problem hiding this comment.
Please use search-match just so we're consistent in our query matching. If we later improve our matching it would improve everywhere. You can also just return it directly.
tests/acceptance/view-tree-test.js
Outdated
| let treeNodes = findAll('.js-view-tree-item'); | ||
| assert.equal(treeNodes.length, 3, 'expected some tree nodes'); | ||
|
|
||
| fillIn('.js-filter-views input', 'post'); |
There was a problem hiding this comment.
I didn't realize we had to use await for async helpers because I thought the test framework automatically took care of making it all synchronous. I read the docs again, and is it that the awareness comes from preceding async tasks, rather than ensuring downstream logic is synchronous? (I hope I said that clearly...)
In other words:
asyncFunction();
// Async helper is aware of the above async behaviour and will wait
fillIn();
// But it doesn't make sure the stuff below gets executed in a synchronous way
some = synchronousLogic();
So to fix it, I would have to use either await or andThen?
await fillIn();
// or
andThen(() => { some = synchronousLogic() });
There was a problem hiding this comment.
Yes precisely, you got it. await is used to avoid using andThen down the line.
tests/acceptance/view-tree-test.js
Outdated
| let viewClassNames = []; | ||
| let durations = []; | ||
|
|
||
| function textFor(selector, context) { |
There was a problem hiding this comment.
Can you move this function outside the tests callbacks so we can reuse the same one to avoid duplication?
Refs Issue-677
|
Thanks for being so quick to provide feedback :) |
When dealing with a complicated view tree, it can be cumbersome scrolling through all the views trying to identify the one that is of interest.
To help make finding it easier I added an input field to filter the list that is visible in the scroll view.
It naively looks for the substring to be present in the name of the view to determine a match.
The filter also works with the components included in the view tree.
Demo:
Refs Issue #677
Looking for feedback around contribution guidelines, and if this solution fits the pattern being used in the application. Otherwise also accepting suggestions for improving the implementation.