fix(modal) Implement W3-compliant tabbing on the modal - no tabbing into the background document

[jmwolfe]

This PR enables the bootstrap modal to use tabbing in forms without the tab cycle taking you to the background document. It implements the full standard W3 specification for browser tabbing. Demo is updated to demonstrate tabbing. May want to scale that back if it's "too much about tabbing." But I think it makes for a strong demo and test of the feature. Added a red outline

[pkozlowski-opensource]

@jmwolfe thnx for the PR! Could you squash all the commits into one so it is easier to review? Thnx!

[jmwolfe]

Not sure what you mean... how do I do that? do another branch, copy files in from the first-round branch and recommit?

[pkozlowski-opensource]

The easiest (?) way would be (from top of my head):

• interactive rebase: git rebase -i HEAD~3
• force-push afterwards:git push --force

[jmwolfe]

OK... got this:
C:\Users\tqgyso0\Documents\GitHub\bootstrap [modal_bg_notab_again +0 1 -0]> git
rebase -i HEAD3
Successfully rebased and updated refs/heads/modal_bg_notab_again.
C:\Users\tqgyso0\Documents\GitHub\bootstrap [modal_bg_notab_again]> git push --f
orce
Everything up-to-date
C:\Users\tqgyso0\Documents\GitHub\bootstrap [modal_bg_notab_again]>

[jmwolfe]

I tried consolidating... what's the next step? is it stuck in pull-purgatory? ⌚

[mbcooper]

Any notion when this might appear in a stable build of Angular-ui/bootstrap?

[jmwolfe]

bump @pkozlowski-opensource -- do I need to do something else to get this in? I used a good chunk of my holiday to get this ready as there seemed to be some healthy demand for it. Do I really need to create a new branch, pull all my changes over to it, and do it in only a single commit? The few commits i have aren't that complicated I don't think.

[aaronkaka]

+1

[mbcooper]

+1

[gindulis]

+1

[db6edr]

+1

[mbcooper]

Any idea when this might be merged?

[aaronkaka]

^^^ this

[mbcooper]

+1
On Aug 11, 2014 4:53 AM, "Dirk Räder" [email protected] wrote:

+1

—
Reply to this email directly or view it on GitHub
#2461 (comment).

[kevinoid]

Thanks for working on this! I'm looking forward to seeing this fixed.

However, as a non-contributor, I'm a little concerned that this approach reimplements the sequential focus navigation algorithm and takes over focus navigation from the browser. Although the implementataion is very good, the approach seems very error prone and likely to need updating whenever new focus targets are added to one of the HTML standards. Also, as noted in the source comments, it doesn't handle dynamic changes to focusable elements (e.g. if readonly/disabled/visibility/etc. are dynamic).

Is there a reason why the upstream Bootstrap approach of listening for focusin and calling focus() whenever the target is not a child of the top modal was not used? Granted it doesn't currently appear to handle Shift-Tab cycling correctly, but that's probably solvable.

[jdanyow]

+1

[omniscient]

+1

[chrisirhc]

@jmwolfe Sorry, I'm not sure about this PR in its current state because it is very different from what I discussed with you on #738 .
@kevinoid raised a valid point, is there a reason why we're not using the simple focusin handler approach? Why do we need to reimplement "tab" events?

[jmwolfe]

OK. I see what happened. I looked into the focusin approach, and actually started that way. but bootstrap 3's implementation of this seemed really JQuery heavy. I wasn't sure about trying to engineer it without JQuery. I went off of @hall5714 's comment ( #738 (comment)) and did a tab-override approach. I noted that @chrisirhc noted we could ignore tabindex and made a reference to focusin but I must have read it too quickly and didn't notice the fact I wasn't dealing with focusin. I asked for feeback on the approach, but nobody had anything to say about it (unfortunately until after it was polished and tested, etc. :( ).

I guess it's back to the drawing board. I have more DOM event experience at this point; perhaps I can build it again around focusin.

Until then, if anyone really needs an implementation, the one on the named fork works perfectly. :) And really, how soon will the tabbing specification change?

Closing this PR.