Media modal: When changing a setting, tabbing to the next field doesn't work

[ocean90]

Tabbing from one to another settings field doesn't work, when the setting was changed.

First reported here: ​http://wordpress.org/support/topic/tabbing-between-titlecaption-on-add-media-screen-not-working?replies=1

[koopersmith]

Yeah. We're not great at persisting the :focus state, and if you change the value, it currently re-renders everything. See also #22502.

I would like to fix this, but I'm not sure we'll be able to do it for 3.5.

[nacin]

This is quite complicated, but here's the basic workflow:

• On the change event, we can record A) which setting is getting changed, and B) if it was a "Tab" that triggered the change event.
• After the view is re-rendered, we can find the field that got changed.
• If it was a tab or shift-tab, we find the next or previous tabbable element (courtesy of jQuery UI) and set the focus there.

There's a lot of work in #22502 to make the entire modal tabbable. That is *not* the goal here. We can cherry-pick what we need from there. The goal here is to simply make it possible to tab from title, to caption, to alt, etc., in that one attachment view settings pane.

Needs an owner, not koopersmith. He's got other things. :-) lessbloat?

[lessbloat]

Replying to nacin:

This is quite complicated, but here's the basic workflow:

• On the change event, we can record A) which setting is getting changed, and B) if it was a "Tab" that triggered the change event.

I started working on this. Ran into a pesky little problem right away where updateSetting() does not pass along any "key" info (i.e. event.keyCode, event.shiftKey, or event.which). I'm assuming because it's actually a change event and not a key up/down event. But this makes part B problematic. Thoughts on any way around this?

[lessbloat]

Replying to lessbloat:

I started working on this. Ran into a pesky little problem right away where updateSetting() does not pass along any "key" info (i.e. event.keyCode, event.shiftKey, or event.which). I'm assuming because it's actually a change event and not a key up/down event. But this makes part B problematic. Thoughts on any way around this?

Nevermind, I attached a keydown even to updateSetting() on top of the change event, and that appears to work.

[lessbloat]

22659.diff​ attempts to fix this. Thanks to Koopersmith for helping me along at times.

[nacin]

Do we need to declare var tabNext at some point in case it is not defined and we try checking for it?

[SergeyBiryukov]

Tabs seem off in lines 2535 and 3731. Looks like the adjacent lines use spaces.

[lessbloat]

Replying to SergeyBiryukov:

Tabs seem off in lines 2535 and 3731. Looks like the adjacent lines use spaces.

I see that in the diff, but it looks fine in my text editor (coda). Weird...

[ryan]

Needs refresh.

[lessbloat]

22659.refresh.diff​ also fixes tabbing in comment:9.

[ryan]

Working well for me.

[helenyhou]

Seems to work for me as well.

[nacin]

To koop for review.

[koopersmith]

The first patch only worked for the attachment details, but not for the compat view. It also applied to every attachment, and couldn't be applied to other views.

Patch adds a FocusManager that can easily be attached to a view to help handle focusing elements on render. It tracks the currently focused element, so a re-render will always attempt to restore the correct focus (whether the tab key was clicked or not).

[helenyhou]

22659.2.diff​ worked for me on a quick test.

[lessbloat]

22659.2.diff​ works for me on Mac Chrome and FF. Win IE7 & IE8 didn't seem to work though (I'm not terribly worried about it though). +1 to 22659.2.diff.

[westi]

New patch works well for me.

[markjaquith]

In 23070:

Implement tabbing between fields in the media modal. props lessbloat, koopersmith. fixes #22659