Fix the issue that non-result node may be chosen. #33

Closed
wants to merge 5 commits into
from

Conversation

Projects
None yet
2 participants
Contributor

hedgerwang commented Aug 31, 2012

When press ENTER to select an item, the result item should be queried lively, instead of beding read from this._display which may also contain non-result nodes.

@hedgerwang hedgerwang Update src/lib/control/typeahead/Typeahead.js
Fix the issue that non-result node may be chosen.
5d5b11b
Owner

epriestley commented Sep 4, 2012

Sorry for the delay on this -- can you give me a little more context? How can _display end up with non-result nodes -- you're intercepting the 'show' event and adding some section dividers or something?

If the other nodes are section dividers, it seems like it would make more sense to prevent these elements from getting focus in the first place. Is there a reason they're allowed to gain focus? Can we make a change to _changeFocus() instead so that we just skip over them?

Contributor

hedgerwang commented Sep 4, 2012

Yes.

The _display contains a list of nodes and some of the nodes are not results nodes. They are dividers or labels.
I've noticed that the 'show' event does expose the _dispose object and let people mutate it, and its name doesn't suggest that it's for result only.

What I did not understand is that why would it assume that _display only contains result nodes since it has make it mutable to others.

I'll see what I can do with the _changeFocus() and related functions.

Thanks.

Contributor

hedgerwang commented Sep 4, 2012

It seems obvious that this._display should only refer to the result nodes.
I've updated showResults() so that non-result nodes can be rendered but won't be referenced by this._display.
Please take another look, thanks.

Owner

epriestley commented Sep 5, 2012

Cherry-picked as c59b8ba.

epriestley closed this Sep 5, 2012

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment