? ? Pending

User tests: Successful: Unsuccessful:

avatar Hackwar
Hackwar
27 Jun 2018

The JS in com_finder in 4.0 is very broken. There are several issues:

  • When typing in the input field, it looses focus very quickly, makeing input nearly impossible.
  • When typing quickly and then sending the form before the suggestions ajax request is fulfilled, an empty error message is shown shortly.
  • Suggestions are displayed for the previous term in the input field. So if you are typing the word "francais" and so far got "fra" and then press "n", the suggestions will retrieve the suggestions for "fra" and not for "fran". Likewise when deleting a character. So when you delete the "n" of "fran", you will get suggestions for "fran" instead.
  • Also sometimes the suggestions aren't displayed at all.

This PR fixes all of these things. I first updated the awesomplete suggestion script with the current head version from its repo (https://github.com/LeaVerou/awesomplete). The last release is from 2017 and we already have that in our codebase, which currently creates the focus errors.

I then created the awesomplete object just once and attached it to the input object, instead of creating it on each ajax call. Now we just update the list instead.

The event for the suggestions update is now upon "keyup", which then now takes the right string for the suggestions.

There is a bit of code that seems to be from a time where the placeholder attribute wasn't present. I removed that.

And last but not least, the error case for the ajax call first checks if there is any status code given. When we are just aborting the ajax call because we are submitting the form, then the status code is 0, otherwise it would be something like 200, 404 or 500.

Happy testing. Hope this makes you happy, @infograf768 and @carlitorweb ?

avatar Hackwar Hackwar - open - 27 Jun 2018
avatar Hackwar Hackwar - change - 27 Jun 2018
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 27 Jun 2018
Category JavaScript
avatar Hackwar Hackwar - change - 27 Jun 2018
Labels Added: ?
avatar dgrammatiko
dgrammatiko - comment - 27 Jun 2018

@Hackwar if you need to do changes on a vendor file you need to do them upstream!
This cannot be accepted because the npm install is removing the media/vendor folder and recreates it from the relative sources. So this is a no go!

Also the package.json dependencies have a format npm name: version so putting there some string is not gonna work.

Just do a npm i or node build.js --update and see what you are breaking here

avatar Hackwar
Hackwar - comment - 27 Jun 2018

package.json supports github repo links, which is what I did. I also did not modify the awesomplete files, but simply took the current head of their github repo. Considering that the last commit that I did to this PR is the result of npm install, I'm pretty confident with this. BTW: When doing an npm install on the current head of 4.0-dev, there is quite a few things that are being changed. So maybe you should again update this.

avatar infograf768
infograf768 - comment - 27 Jun 2018

?
Focus back and suggestions look fine with my first tests.

avatar infograf768
infograf768 - comment - 27 Jun 2018

Now, just please solve the highlighting for non-ascii stuff. ;)

avatar Hackwar
Hackwar - comment - 27 Jun 2018

Did that in #20571 ?

avatar infograf768 infograf768 - test_item - 28 Jun 2018 - Tested successfully
avatar infograf768
infograf768 - comment - 28 Jun 2018

I have tested this item successfully on 011bbab


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/20888.

avatar carlitorweb
carlitorweb - comment - 28 Jun 2018

nice one!!!

avatar carlitorweb carlitorweb - test_item - 28 Jun 2018 - Tested successfully
avatar carlitorweb
carlitorweb - comment - 28 Jun 2018

I have tested this item successfully on 011bbab


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/20888.

avatar franz-wohlkoenig franz-wohlkoenig - change - 28 Jun 2018
Status Pending Ready to Commit
avatar franz-wohlkoenig
franz-wohlkoenig - comment - 28 Jun 2018

Ready to Commit after two successful tests.

avatar wilsonge wilsonge - change - 1 Jul 2018
Status Ready to Commit Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 2018-07-01 13:52:06
Closed_By wilsonge
Labels Added: ?
avatar wilsonge wilsonge - close - 1 Jul 2018
avatar wilsonge wilsonge - merge - 1 Jul 2018

Add a Comment

Login with GitHub to post a comment