Bulk edit tag autocomplete layout error

[davidbenton]

When bulk editing tags (or any taxonomy terms), the autocomplete suggestions <ul> is positioned correctly when it is first opened, but closing and reopening the bulk edit form (by using the "Cancel" button, for example) causes the term autocomplete list to be positioned incorrectly. See image:
​https://www.dropbox.com/s/c5iragizvk3wak3/Screenshot%202017-03-23%2012.46.19.png?dl=0

I was able to consistently reproduce this in latest Chrome and Safari on OS X for v4.7.2 and v4.7.3.

[afercia]

@thanks @davidbenton, I was able to reproduce it in Firefox too, seems something related to jQuery ui autocomplete not being able to correctly calculate the left position when the Quick Edit form gets closed and then re-opened. Introduced in 4.6 see #33902. Would be nice to try to fix it in the next minor release, but I will leave the decision to the release lead of course.

Re-posting your screenshot to make it immediately visible:

[davidbenton]

Changing tags-suggest.js near line 153 from:

			position: {
				my: 'left top+2'
			},

to...

			position: {
				my: 'left top+2',
				at: 'left bottom'
			},

...solves for me. I'll work on submitting a patch, but I'm new around here so it may take a bit for me to figure out the right way to do that, and I wanted to go ahead and post a potential solution.

[davidbenton]

Man, it's been a while since I used svn... hope that patch is right. :) I've tested the functionality in a couple browsers, and jQuery UI's docs on this seem straightforward, but would love feedback from someone who has more context for WP admin's inner workings.

[afercia]

@davidbenton yep same idea here, ensuring the defaults are always passed fixes it for me. Not sure why it doesn't happen for quick edit though (still have to dig into that part). I'd just make sure to pass also collision.

[afercia]

Dumping the options object in wp-includes/js/jquery/ui/position.js right after options = $.extend( {}, options ); highlights that, the second time Bulk Edit is open, the default are not extended but overridden instead.

[afercia]

To clarify, jQuery UI autocomplete uses the following defaults (which are different from the ​jQuery UI position defaults):

position: {
	my: "left top",
	at: "left bottom",
	collision: "none"
},

[afercia]

Tested a bit more and I think the different behaviour between Bulk Edit (where the misplacing occurs) and Quick Edit (which works ok) depends on a mix of factors.
Both Quick Edit and Bulk Edit use a hidden form that then gets revealed. By the way, the Quick Edit one gets cloned each time with jQuery clone() so it's a new object each time while the Bulk Edit one gets just moved through the DOM. I guess the Quick Edit autocomplete widget is always a new fresh instance and thus its options are properly extended, while the Bulk Edit autocomplete widget gets its options overridden the second time it gets used.
Regardless, passing always a complete position object solves the positioning issue. Patch looks good to me. @davidbenton any thoughts? :)

[davidbenton]

Yep, looks good to me @afercia, and works for me.

Thanks for digging deeper. Interesting about the differences between Quick Edit and Bulk Edit. Almost sounds like it may be a jQuery UI issue, or do you think Bulk Edit is using it in an unusual way? I will try to take a look at that myself, but probably won't be today.

I think the issue as described in the ticket is fixed by the patch, though, so as far as I'm concerned this can move forward as-is. Thanks!

[azaozz]

Replying to afercia:

40242.diff works properly and looks good here too.

[afercia]

Replying to davidbenton:

Almost sounds like it may be a jQuery UI issue, or do you think Bulk Edit is using it in an unusual way?

Maybe a mix of both :) Yes, autocomplete here is used in a particular way. While Quick Edit uses each time a new fresh autocomplete instance, Bulk Edit re-uses the same instance.

On the other hand what autocomplete does when the suggestions get displayed doesn't work for some edge cases. It seems to me it defeats the purpose of extending the defaults with the passed options, since the default position object there is incomplete (there's just the of property). This could be confusing for developers because when using .extend() we should safely assume there are always some proper defaults, even when using autocomplete in a particular way.

Maybe worth considering to check if an autocomplete widget instance already exists and avoid calling again wpTagsSuggest() in that case. /cc @azaozz

[afercia]

While 40242.diff​ works, 40242.2.diff​ also avoids to call again wpTagsSuggest() when an autocomplete widget instance already exists for the Bulk Edit textarea. This makes passing the at and collision properties not strictly necessary but I'd say to keep them, just in case...

[davidbenton]

I found a ​jQuery UI ticket addressing this, and it was closed as "not a bug." So, while I find that behavior less than intuitive, I guess it is as intended by jQuery UI.

Replying to afercia:

40242.2.diff​ [...] makes passing the at and collision properties not strictly necessary but I'd say to keep them, just in case...

I think that's reasonable. It makes the position more explicit and readable, which I'd consider valuable even if those are the defaults, especially given the potential confusion of different defaults for jQuery UI and jQuery UI autocomplete.

[afercia]

In 40357:

Quick/Bulk Edit: Fix the Tag suggestions position on the Bulk Edit textarea.

Always passes the complete position object to the jQuery autocomplete widget.
Also checks if an autocomplete instance already exists on the Bulk Edit textarea.

Props davidbenton.
Fixes #40242.

[afercia]

Re-opening for 4.7.4 consideration.

[swissspidy]

In 40365:

Quick/Bulk Edit: Fix the Tag suggestions position on the Bulk Edit textarea.

Always passes the complete position object to the jQuery autocomplete widget.
Also checks if an autocomplete instance already exists on the Bulk Edit textarea.

Props davidbenton.
Fixes #40242.

Merges [40357] to the 4.7 branch.