Autocomplete improvement - replacing data (AJAX), arrows keys usage and more #3694

nuplo · 2016-09-27T08:59:28Z

Hi,
aoutocomplete in official version doesn't have a lot of crucial features so I decided to add:
-up and down arrows usage
-replacing data for autocomplete list - for instance for AJAX request
-if auto complete list is visible and you will make focusout action from input field, whole list disapper. I think in most cases we don't want to display this list if we do something else. It's rather bug than feature.
-when you write something in auto complete field and press "Esc" button whole list will disappear
-whole content after autocomplete list go down when list is visible. I changed list position on absolute and now it doesn't affect other elements
-in my opinion this list is sometimes too long - for example if we have 100 elements. Because of that, I decided to extend default object and add "maxElementsAmount" option. If you set that for example on 5 during initialization, you will see only 5 elements from the top of this list. If you skip that, you will see all matching elements.

Please check that, if you have any questions or comments just write. I'm hope so we can marge that soon because I use that in my project ;)

Cheers,
Adam

akatriel

Changes look good and work well. Tested on a local project.

twoe-hub · 2016-10-04T07:08:24Z

Works well; adapted this in a commercial project.

arlowhite · 2016-10-05T17:25:34Z

Also, I wanted a 'select' event for when the user clicks an entry in the autocomplete or presses enter. ('change' fires for every key)

What are your thoughts on a 'select' event?

See #3729

arlowhite

Looks pretty good overall. I don't have time to test the code right now, but here are a few things I noticed scanning the code on GitHub.

arlowhite · 2016-10-05T17:27:37Z

js/forms.js

          });
+
+          // updating input value
+          setIputValue($autocomplete, $input);


Typo. should be setInputValue

arlowhite · 2016-10-05T17:28:34Z

js/forms.js

-        var data = options.data,
-            $inputDiv = $input.closest('.input-field'); // Div to append on
+      // Set input value
+      function setIputValue(ulElement, inputElement) {


Why setIputValue instead of setInputValue?

You're right I changed that.

arlowhite · 2016-10-05T17:36:17Z

js/forms.js

-        var $input = $(this);
-        var data = options.data,
-            $inputDiv = $input.closest('.input-field'); // Div to append on
+      // Set input value


The JavaDoc doesn't give more information than already hinted by the name of the function.

Is inputElement the main $input? Why is the parameter needed? Can't you just use $input captured in the closure?

Why is this function needed? It's only used once in the code. I think just remove the function and put its body where the call on line 450 is.

Comment maybe doesn't bring so much, but I do that because it's easier for me to read a code. Comments have different color and for me when I looking through the code it's like headers.

I separated that intentionally. First and less important I think right now it's just more readable. In one section you define functions - in different section you fired that.
But more important thing: if you move everything to one loop and have for example 100 autocomplete inputs, when you run this loop you will define all this function 100 times. Right now you will define functions only once and you can fire that 100 times. It's just about performance.

And you cannot move this code to line 450 because this function just will not work.

arlowhite · 2016-10-05T17:37:11Z

js/forms.js

+      // highlight elements
+      function highlight(string, $el) {
+        var img = $el.find('img');
+        var matchStart = $el.text().toLowerCase().indexOf("" + string.toLowerCase() + ""),


No reason to add empty strings, it doesn't do anything.

it's a legacy after old code. I changed that and I'm hope so it's not a some kind of hack for old IE or something.

arlowhite · 2016-10-05T17:45:22Z

js/forms.js

+        // prevent refreshing list on pressing ctrl, shif, alt, arrow, f1-f12, etc
+        // helpful if we selected something using arrow and we press ctrl for example
+        if(
+        [9, 16, 17, 18, 19, 20, 33, 34, 35, 36, 37, 38, 39, 40, 45, 112, 113, 114, 115, 116, 117, 118, 119, 120, 121, 122, 123].indexOf(e.which) < 0


Alphanumeric codes are probably in the same range. So probably better to do if(e.which > X && e.which < Y)

But I have no idea what X and Y should be. I started looking into the docs, but they don't mention numerical codes.
https://api.jquery.com/event.which/
https://www.w3.org/TR/uievents-code/

I'm not so sure about that, but if you could provide solution like that and it will work we can use that.

arlowhite · 2016-10-05T17:49:40Z

js/forms.js

+            if (data.hasOwnProperty(key) &&
+            key.toLowerCase().indexOf(val) !== -1 &&
+            key.toLowerCase() !== val) {
+              if (maxElementsAmount !== null) {


I think the code should establish that maxElementsAmount is a number before starting the for loop rather than checking if it is null here.

Hmmm... If you do that you can change:
if (maxElementsAmount !== null)
On something like this:
if (maxElementsAmountIsNumber === true)

I don't think it's a big change

arlowhite · 2016-10-05T17:51:44Z

js/forms.js

+      function arrowUsage(e, $autocomplete) {
+        // Capture up and down key
+        if (
+        e.which === 38


Maybe save e.which in var since it's used a few times.
var keyCode = e.which;

arlowhite · 2016-10-05T17:53:44Z

js/forms.js

+      // arrow usage - on key down
+      function arrowUsage(e, $autocomplete) {
+        // Capture up and down key
+        if (


if statement line breaks and indenting look weird. (Not normal JavaScript style)

I changed that, but all programmers has own style and there isn't "the style" which you can use in each situation. It's about preferences. If we want to have the same style in each file we should configure task in "gulp watch" which will be reformating code after save. That's the only way to have the same style in the whole project if you have more than 2 programmers ;)

arlowhite · 2016-10-05T17:53:56Z

js/forms.js

+        }
+
+        // Check if data isn't empty and append autocomplete element if doesn't exist yet - run only once
+        if(


if statement line breaks and indenting look weird. (Not normal JavaScript style)

arlowhite · 2016-10-05T18:00:08Z

sass/components/forms/_input-fields.scss

-  position: static;
+  position: absolute;
+
+  & ~ .autocomplete-content,


CSS changes should be reviewed, but I don't have time to look at them now.

I've just found one surplus line and deleted it but it doesn't change so much.

nuplo · 2016-10-06T10:56:44Z

@arlowhite I added callback onSelect which you described in #3729

…wn event. It generated some issues for validation in some cases.

gdoron · 2016-12-20T12:30:24Z

📢 📢 📢 Why things in this project never get merged?
What's the purpose of having an open source project if people's hard work is flush into the toilet every time?
@Dogfalo don't get me wrong, I really appreciate your hard work, but please let the community help you maintain this amazing project!

Adam Basa added 2 commits September 27, 2016 16:10

js and scss file done for autocomplete

0a4afd1

documentation updated

2982b06

nuplo mentioned this pull request Sep 27, 2016

autocompete with ajax #3539

Closed

replacing data improvment - now it works :)

0aa4be8

This was referenced Sep 27, 2016

Added option to rewrite data in autocomplete #3646

Closed

Dropdown not accessible with arrow keys #3502

Closed

literal error in documentation

5dcd01f

nuplo changed the title ~~Autocomplete improvement - replacing data, arrow keys usage, etc~~ Autocomplete improvement - replacing data (AJAX), arrows keys usage and more Sep 28, 2016

akatriel approved these changes Sep 28, 2016

View reviewed changes

arlowhite mentioned this pull request Oct 5, 2016

Add showSingleMatch option to autocomplete. #3734

Closed

arlowhite reviewed Oct 5, 2016

View reviewed changes

Adam Basa added 2 commits October 6, 2016 18:17

changes according to @arlowhite's code review

1fe49ae

callback on select element event

e84c1e6

nuplo mentioned this pull request Oct 6, 2016

Add 'select' event to autocomplete #3729

Closed

gbsuar mentioned this pull request Oct 25, 2016

change autocomplete for ajax requests #3541

Closed

Small improvement - I moved enter key usage from keyup event to keydo…

119f9d5

…wn event. It generated some issues for validation in some cases.

tomscholz added the component:Autocomplete label May 29, 2017

This pull request was closed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Autocomplete improvement - replacing data (AJAX), arrows keys usage and more #3694

Autocomplete improvement - replacing data (AJAX), arrows keys usage and more #3694

nuplo commented Sep 27, 2016 •

edited

Loading

akatriel left a comment

twoe-hub commented Oct 4, 2016

arlowhite commented Oct 5, 2016

arlowhite left a comment

arlowhite Oct 5, 2016

nuplo Oct 6, 2016

arlowhite Oct 5, 2016

nuplo Oct 6, 2016

arlowhite Oct 5, 2016

nuplo Oct 6, 2016

arlowhite Oct 5, 2016

nuplo Oct 6, 2016

arlowhite Oct 5, 2016

nuplo Oct 6, 2016

arlowhite Oct 5, 2016

nuplo Oct 6, 2016

arlowhite Oct 5, 2016

nuplo Oct 6, 2016

arlowhite Oct 5, 2016 •

edited

Loading

nuplo Oct 6, 2016

arlowhite Oct 5, 2016 •

edited

Loading

nuplo Oct 6, 2016

arlowhite Oct 5, 2016

nuplo Oct 6, 2016

nuplo commented Oct 6, 2016

gdoron commented Dec 20, 2016

Autocomplete improvement - replacing data (AJAX), arrows keys usage and more #3694

Autocomplete improvement - replacing data (AJAX), arrows keys usage and more #3694

Conversation

nuplo commented Sep 27, 2016 • edited Loading

akatriel left a comment

Choose a reason for hiding this comment

twoe-hub commented Oct 4, 2016

arlowhite commented Oct 5, 2016

arlowhite left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

arlowhite Oct 5, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

arlowhite Oct 5, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nuplo commented Oct 6, 2016

gdoron commented Dec 20, 2016

nuplo commented Sep 27, 2016 •

edited

Loading

arlowhite Oct 5, 2016 •

edited

Loading

arlowhite Oct 5, 2016 •

edited

Loading