Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Fix autocomplete issue with latest jquery UI in 1.2 stable #2408

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
3 participants

jphpsf commented Jan 9, 2013

I stumbled upon this issue today. This was after upgrading to Spree 1.2.3 yesterday.

Step to reproduce: in the Spree admin, go to the Orders tab and create a New Order. Click Customer Details in the sidebar and search for a customer in the Customer Search area. When the autocomplete results appear click on the email address of a customer (note: this is kind of a edge case and you must click exactly on the <h4> tag containing the email address).

Expected behavior: the customer should be selected and the form should be populated

Broken behavior: the customer is not selected, the form stays empty and the javascript console shows the following error Uncaught TypeError: Cannot read property 'label' of undefined multiple times.

Workaround: only mouse click is affected by the issue, so as a workaround a user can use the keyboard to navigate and select the autocomplete (arrow up/down and enter keys)

Cause: the problem comes from a change in jQuery UI that is not exactly compatible with Spree's customer autocomplete. When I upgraded to Spree 1.2.3 / Rails 3.2.10 yesterday, bundler also updated the dependency for jquery-rails:

-    jquery-rails (2.1.3)
+    jquery-rails (2.1.4)

The minor version bump actually bumps the version of jQuery UI from 1.8.23 to 1.9.2 (see list of changes or this specific commit). The autocomplete in jQuery UI got a bit of a refactor and it seems that the click handling is not exactly the same.

The autocomplete click handler looks up the closest element with a class .ui-menu-item. It is expected to be a <li> tag (the class is actually added by jQuery on the <li> tag containing an autocomplete result item).

Now let's look at the autocomplete in Spree: the following line is the culprit. Spree autocomplete customize the rendering of each autocomplete result and produces the following markup:

<li class="ui-menu-item">
  <a class="ui-menu-item ui-corner-all">
    <h4>spree@example.com</h4>
    <span><strong>Billing: xxxxxx</strong> </span>
    <span><strong>Shipping: xxxxxx</strong> </span>
  </a>
</li>

The class .ui-menu-item is added to the <a> tag inside the <li>. So if the user is clicking on the <h4>, the closest match becomes the containing <a> tag instead of the expected <li> and the javascript error is triggered causing the customer to not be selected. Same thing happen if clicking on shipping or billing addresses. The only way to get the click working is to click inside the <a> tag but outside of the child elements.

Fix: I did 2 things: first I changed the capybara test case to demonstrate the bug. I made the test case click the <h4> instead of the the <a>. I also remove the mouseenter interaction as this triggers the autocomplete selection and hides the bug. Then, I fix the bug by removing the extra .ui-menu-item class from the <a> tag.

Branch affected: this only affect the 1.2 branch as >= 1.3 uses select2 instead of jQuery UI autocomplete.

cduv pushed a commit to cduv/spree that referenced this pull request Jan 16, 2013

jphpsf commented Jan 21, 2013

Note that your recent commit 3e476ba on the 1-2-stable branch fixes this issue too. I am assuming that the 1-2-stable will stay on the same jQuery version, so my patch might not be needed after all. Cheers!

@radar radar closed this Jan 21, 2013

fabiode commented Feb 21, 2013

I believe I'm having the same error in 1.1.5 too. I'll try to fix like you did.

jphpsf commented Feb 22, 2013

@fabiode If you pick up the change from Ryan that would work. See commit https://github.com/spree/spree/blob/1-1-stable/core/spree_core.gemspec

All you should need is to freeze the jquery-rails gem to 2.1.4. Feel free to also create a pull request so that others can benefit from your fix :) 👍

fabiode commented Feb 22, 2013

work it like a charm!

thanks!

Member

radar commented Feb 28, 2013

jquery-rails is restricted to 2.1.4 in 1-2-stable, so I think this problem
is fixed now? Please yell and scream if it's not.

On Fri, Feb 22, 2013 at 12:23 PM, Fabio Daguer Esposito <
notifications@github.com> wrote:

work it like a charm!

thanks!


Reply to this email directly or view it on GitHubhttps://github.com/spree/spree/pull/2408#issuecomment-13956689.

Ryan Bigg
Community Manager
Spree Commerce, Inc.

Register now for SpreeConf
May 20-21 in Washington, D.C.
http://spreeconf.com

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