Skip to content
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.

Sign up for GitHub

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

Jump to bottom

View tree search filter #720

Merged
merged 6 commits into from
Oct 7, 2017
Merged

View tree search filter #720

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
c64278e
MVP of the search filter for the view tree
kamikaz1k Oct 6, 2017
ca0dbfb
Moved search box to view-tree-toolbar
kamikaz1k Oct 7, 2017
f719298
Added acceptance test for view-tree search
kamikaz1k Oct 7, 2017
fa01fe4
Cleaned up debug code
kamikaz1k Oct 7, 2017
b3575bc
Cleaned up filtering function
kamikaz1k Oct 7, 2017
f734ad7
Included PR feedback for filtering
kamikaz1k Oct 7, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions app/controllers/view-tree.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,19 @@ export default Controller.extend({
components: false
},

searchText: "",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 ''
   */


filteredList: computed('model', 'searchText', function() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

computed('model.[]', 'searchText'

  /**
   * The filtered view list.
   * 
   * @property filteredList
   * @type {Array<Object>}
   */

let searchText = this.get('searchText');

if (!searchText) {
return this.get('model');
}

let filtered = this.get('model').filter(v => v.value.name.includes(searchText));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

return filtered;
}),

optionsChanged: on('init', observer('options.components', function() {
this.port.send('view:setOptions', { options: this.get('options') });
})),
Expand Down
6 changes: 6 additions & 0 deletions app/templates/view-tree-toolbar.hbs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,10 @@
<div class="toolbar__checkbox js-filter-components">
{{input type="checkbox" checked=options.components id="options-components"}} <label for="options-components">Components</label>
</div>

<div class="divider"></div>

<div class="toolbar__search js-filter-views">
{{input type="text" id="options-views" placeholder="Search Views" value=searchText}}
</div>
</div>
2 changes: 1 addition & 1 deletion app/templates/view-tree.hbs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{{#x-list name="view-tree" schema=(schema-for "view-tree") as |list|}}
{{#list.vertical-collection content=model as |content index|}}
{{#list.vertical-collection content=filteredList as |content index|}}
{{view-item
model=content
inspect=(action "inspect")
Expand Down
68 changes: 67 additions & 1 deletion tests/acceptance/view-tree-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import Ember from "ember";
import { test } from 'ember-qunit';
import { module } from 'qunit';
import startApp from '../helpers/start-app';
import { visit, find, findAll, click, triggerEvent } from 'ember-native-dom-helpers';
import { visit, fillIn, find, findAll, click, triggerEvent } from 'ember-native-dom-helpers';

let App;
const { run } = Ember;
Expand Down Expand Up @@ -118,6 +118,7 @@ test("It should correctly display the view tree", async function(assert) {

let treeNodes = findAll('.js-view-tree-item');
assert.equal(treeNodes.length, 3, 'expected some tree nodes');

let controllerNames = [];
let templateNames = [];
let modelNames = [];
Expand Down Expand Up @@ -186,6 +187,71 @@ test("It should correctly display the view tree", async function(assert) {
], 'expected title tips');
});

test("It should filter the view tree using the search text", async function(assert) {
let viewTree = defaultViewTree();

await visit('/');
run(() => {
port.trigger('view:viewTree', { tree: viewTree });
});
await wait();

let treeNodes = findAll('.js-view-tree-item');
assert.equal(treeNodes.length, 3, 'expected some tree nodes');

fillIn('.js-filter-views input', 'post');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

await

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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() });

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes precisely, you got it. await is used to avoid using andThen down the line.

treeNodes = findAll('.js-view-tree-item');
assert.equal(treeNodes.length, 1, 'expected filtered tree nodes');

let controllerNames = [];
let templateNames = [];
let modelNames = [];
let viewClassNames = [];
let durations = [];

function textFor(selector, context) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you move this function outside the tests callbacks so we can reuse the same one to avoid duplication?

return find(selector, context).textContent.trim();
}

[...treeNodes].forEach(function(node) {
templateNames.push(textFor('.js-view-template', node));
controllerNames.push(textFor('.js-view-controller', node));
viewClassNames.push(textFor('.js-view-class', node));
modelNames.push(textFor('.js-view-model', node));
durations.push(textFor('.js-view-duration', node));
});

assert.deepEqual(controllerNames, [
'App.PostsController',
], 'expected controller names');

assert.deepEqual(templateNames, [
'posts',
], 'expected template names');

assert.deepEqual(modelNames, [
'PostsArray',
], 'expected model names');

assert.deepEqual(viewClassNames, [
'App.PostsView',
], 'expected view class names');

assert.deepEqual(durations, [
'1.00ms',
], 'expected render durations');

let titleTips = [...findAll('span[title]')].map(node => node.getAttribute('title')).sort();

assert.deepEqual(titleTips, [
'App.PostsController',
'App.PostsView',
'PostsArray',
'posts',
'posts'
], 'expected title tips');
});

test("It should update the view tree when the port triggers a change", async function(assert) {
assert.expect(4);
let treeNodes, viewTree = defaultViewTree();
Expand Down