Skip to content

Conversation

clarabstract
Copy link

When any kind of DOM manipulation occurs (replaceNode, insertChild etc), any elements being manipulated loose their focus status (as of Chrome 12, Firefox 5). I couldn't find any spec that clarifies why that is, but it does appear to happen consistently.

IMO, it's reasonable to expect end-users to deal with this during explicit DOM manipulation calls ($.html(), $.after(), etc), but not necessarily so during an animation. The fact that jquery.ui animations end up performing DOM manipulations implicitly is not at all obvious to the user and thus the focus-loosing behavior is unexpected and worth avoiding.

This is probably not a very common use case but $.effects.removeWrapper really is the only place to fix it (there are no callbacks in which to detect the previously focused element) and the overhead doesn't strike me as terribly noticeable considering it only occurs at the end of an animation.

Cheers,

  • Ruy

@gnarf
Copy link
Member

gnarf commented Jul 30, 2011

Hrm.... Wouldn't the createWrapper() be causing the element to lose focus as well? If so, wouldn't the element no longer be focused by the time that you remove the wrapper?

Also, can you please adhere to the coding standards

Particularly

  • Prefer double quotes always.
  • Spacing inside parens, for the if and function calls that have args, I.E.: if ( element.parent().is( ".ui-effects-wrapper" ) )
  • Commit Message Should be "Effects: reason - Fixes #bug - Bug Desc"
  • If this bug is present in the latest stable release (which I believe it is), file a bug about the behavior or see if one exists already on http://bugs.jqueryui.com

@scottgonzalez
Copy link
Member

This should be using document.activeElement to get the element directly, instead of the :focus selector.

@clarabstract
Copy link
Author

Closing for further discussion in http://bugs.jqueryui.com/ticket/7595 - I ran into complications :/

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

Successfully merging this pull request may close these issues.

3 participants