-
Notifications
You must be signed in to change notification settings - Fork 18
Added options: suggestOnFocus, autocomplete, isAllowSuggestOnEmptyInput #30
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -232,6 +232,21 @@ <h3>{{label}}</h3> | |
value:false, | ||
notify:true | ||
}, | ||
/** | ||
* A boolean property to tell if need to open show up with empty input string. | ||
* isAllowSuggestOnEmptyInput | ||
*/ | ||
isAllowSuggestOnEmptyInput: { | ||
type:Boolean, | ||
value:false | ||
}, | ||
/** | ||
* A boolean property to tell if need to open show up on focusing input element. | ||
*/ | ||
suggestOnFocus: { | ||
type:Boolean, | ||
value:false | ||
}, | ||
/** | ||
* Input value. | ||
*/ | ||
|
@@ -246,6 +261,13 @@ <h3>{{label}}</h3> | |
type:Object, | ||
notify:true | ||
}, | ||
/** | ||
* Bind this to the <paper-input>'s autocomplete property. | ||
*/ | ||
autocomplete:{ | ||
type:String, | ||
value:'address-level4' | ||
}, | ||
/** | ||
* Bind this to the <paper-input>'s alwaysFloatLabel property. | ||
*/ | ||
|
@@ -296,6 +318,11 @@ <h3>{{label}}</h3> | |
pattern:String | ||
}, | ||
|
||
listeners: { | ||
'focus': '_onInputFocus', | ||
'blur': '_onInputBlur' | ||
}, | ||
|
||
// Element Lifecycle | ||
|
||
ready: function() { | ||
|
@@ -313,7 +340,7 @@ <h3>{{label}}</h3> | |
} | ||
}); | ||
var input = Polymer.dom(this.root).querySelector('paper-input'); | ||
input.$.input.autocomplete = 'address-level4'; | ||
input.$.input.autocomplete = this.autocomplete; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do you have any use case you think it's necessary to assign this autocomplete value of the input? This line of code is to fix the Chrome Autofilling problem. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Without autocomplete="Off" I have suggest(browser) on On 13 July 2016 at 23:31, Tianxiang Zhang notifications@github.com wrote:
C уважением, Пличко Павел There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To prevent autofilling of Chrome, autocomplete="Off" is not a reliable solution. Chrome Browser Ignoring AutoComplete=Off. See discussion here http://stackoverflow.com/questions/12374442/chrome-browser-ignoring-autocomplete-off. And plus, the expected behavior of paper-typeahead-input is to provide its own suggest, so I don't see why we need to expose this autocomplete property of inner input as the property of paper-typeahead-input. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ок, no problem.But this solution is working for me. On 18 July 2016 at 17:13, Tianxiang Zhang notifications@github.com wrote:
C уважением, Пличко Павел There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. do u mean you are using autocomplete="off"? because based on your current code, it's setting autocomplete="address-level4" still. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here is my usage, that prevents chrome suggesting. <paper-typeahead-input On 18 July 2016 at 17:16, Tianxiang Zhang notifications@github.com wrote:
C уважением, Пличко Павел There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Got it. But if I didn't remember wrong, I have been searching for the chrome suggesting problem a lot before and the conclusion of discussions I found is that autocomplete="Off" is not reliable, workaround is to use autocomplete="address-level4". But it was quite ago, I am not sure if Chrome made some updates to change the situation here. If you like, can you search for some docs or evidence to say autocomplete="Off" is working now? Also, you change it to be "Off" is it because the original implementation using autocomplete="address-level4" is not working? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. On 18 July 2016 at 17:32, Tianxiang Zhang notifications@github.com wrote:
C уважением, Пличко Павел |
||
}, | ||
|
||
attached: function() { | ||
|
@@ -394,7 +421,11 @@ <h3>{{label}}</h3> | |
* @param {e} event | ||
*/ | ||
_keyup: function(e) { | ||
if (e.which == 40){ | ||
if (e.which == 27 && this.suggestOnFocus) { | ||
// esc button | ||
this._clearSuggestings(); | ||
} | ||
else if (e.which == 40){ | ||
//down button | ||
var suggestionsMenu = Polymer.dom(this.root).querySelector('paper-menu'); | ||
var selectedItem = suggestionsMenu.focusedItem; | ||
|
@@ -473,8 +504,8 @@ <h3>{{label}}</h3> | |
} | ||
}, | ||
_search:function(term){ | ||
if (term == ""){ | ||
this._suggestions = []; | ||
if (term == "" && !this.isAllowSuggestOnEmptyInput){ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you give a use case that we need to search on empty string? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This case for suggest items when no chars in the input. When you dont know On 18 July 2016 at 18:06, Tianxiang Zhang notifications@github.com wrote:
C уважением, Пличко Павел There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Okay. This makes sense. And it's good to have it default to be false. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. And also note that based on current implementation, the search will go localCandidates, _prefetchedCandidates and remoteCandidates in order. So for you use case, the top 10 will be the first 10 items in localCandidates. |
||
this._clearSuggestings(); | ||
return; | ||
} | ||
var patt = new RegExp(term.toLowerCase()); | ||
|
@@ -543,8 +574,29 @@ <h3>{{label}}</h3> | |
else{ | ||
this._suggestions = matched; | ||
} | ||
}, | ||
_clearSuggestings:function() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would suggest to rename it to be _clearSuggestions to be consistent with other variables used in the code. And also as the name indicated, it should only clear the suggestions, but not make the input lose focus. So maybe create another _ function to only perform lose focus and call it in e.which = 27. P.S. I tried with twitter typeahead.js when clicks on ESC the input don't lose focus. But when I try with google.com or Google drive, the input loses focus when clicks on ESC. I don't know yet what's the better behavior here. |
||
this._suggestions = []; | ||
var input = Polymer.dom(this.root).querySelector('paper-input'); | ||
input.$.input.blur(); | ||
}, | ||
_onInputFocus:function(e, detail) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't know why when I test locally, the this function _onInputFocus is not triggered when input is focused. And also _onInputBlur is not triggered when lose focus. Do I miss anything to get it working? I already set the suggestOnFocus to be true. ( but this shouldn't matter though. |
||
if ( detail.sourceEvent | ||
&& detail.sourceEvent.relatedTarget == null | ||
&& this.suggestOnFocus | ||
) { | ||
this._search(this.inputValue.trim()); | ||
} | ||
}, | ||
_onInputBlur:function(e, detail) { | ||
if ( detail.sourceEvent | ||
&& detail.sourceEvent.relatedTarget == null | ||
&& this.suggestOnFocus | ||
) { | ||
this._search(this.inputValue.trim()); | ||
this._clearSuggestings(); | ||
} | ||
} | ||
|
||
}); | ||
|
||
</script> |
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.
Both twitter typeahead.js and google.com search has this suggest on focus and clear suggest when lose focus, so I think maybe it's good to have the default value to be true to encourage developer to be consistent with common user experience.