Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

"focus" doesn't bubble #241

Closed
aglemann opened this Issue · 13 comments

2 participants

@aglemann

What am I missing? The code seems to depend on focus bubbling from the anchor inside each list item to the item itself.. but as of at least jQuery 1.6 that doesn't happen. By subclassing the widget and adding a focus handler to each anchor that triggers "focus" on it's parent the widget works perfectly. But otherwise behaves pretty broken.

@fnagel
Owner

Ähh yes. I have no idea what you are talking about. What about a problem description with more than 3 words? Or a line about how to reproduce this? Or the relevant browsers?

See #61

@aglemann

Tks - sorry I didn't see #61. See the jsfiddle I had posted above, that duplicates the issue and includes an example fix. I will add a pull request shortly. This affects all browsers.

@aglemann

Please see pull request #247:

#247

That includes a unit test that duplicates the issue and the patch that fixes the issue.

@fnagel
Owner

Am I right that this issue is directly related to #240?
The problem is that the focus is not set to the link, but it does in previous commits (commits using jQuery 1.6.x), right?

Why didn't you use the prepared fiddle? Like http://jsfiddle.net/fnagel/GXtpC/498/

@fnagel
Owner

I would prefer to understand why the focus doesnt bubble as with jQuery 1.6.x and I think this is not a clean solution. But, after all, it works pretty nice :-)

@fnagel fnagel referenced this issue from a commit
Aeron Glemann fixed: "focus" doesn't bubble, see #241 397e986
@fnagel
Owner

The last unit test you created fails.

@aglemann

Sorry, I posted my jsfiddle before you posted the reference to #61 that included the jsfiddle template.

I agree this solution is hacky. I'd prefer to either trigger all events on the LI or add all the listeners to the A. I'll put together a better fix. Which Browser/Ver/OS did the last test fail on? (ironic i'm asking this).

@fnagel
Owner

No problem, it just had been easier.

I think it would be most suitable if the focus is handled within the anchors, as native lis wont get focus.

Ive tested in latest FF.

Please note I roughly cleaned my branch to get rid of unneded files.

@fnagel
Owner

Any feedback on this issue?

@aglemann

Will get this push request by EOD tomorrow... been swamped.

@aglemann

Please see this pull request:

#257

Includes tests that have have passed in Safari, Chrome, FF, IE7/8.

@fnagel fnagel closed this issue from a commit
Aeron Glemann #241
Updated jquery.simulate.js to latest
Fixed broken path in tests/jquery.js
Updated test to not use jquery.simulate for keypress (simulate was failing, not the test)
Fixed #241 in jquery.ui.selectmenu.js
Added the option object as optional parameter to formatText
3a751b7
@fnagel fnagel closed this in 3a751b7
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.