fix(modal): shift+tab without focused element within modal #5294
Conversation
|
@AndriIushchuk, please fix the broken tests. Thanks. |
|
Also, just a tip, you can run the tests locally without committing them simply by executing the |
|
@icfantv thanks for the tip, tests fixed |
|
Thanks. I'll mark for review as I admit I don't know this code well enough to sign off on it. |
| if (evt && modalWindow) { | ||
| var modalDomEl = modalWindow.value.modalDomEl; | ||
| if (modalDomEl && modalDomEl.length) { | ||
| return (evt.target || evt.srcElement) === modalDomEl[0]; |
There was a problem hiding this comment.
This might need to check whether it contains the element using contains.
There was a problem hiding this comment.
But by using contains, method will return true for each tabbable element within modalWindow
There was a problem hiding this comment.
The name of the function is not quite right if that is the case then.
There was a problem hiding this comment.
How about $modalStack.isModalFocused?
| @@ -606,11 +606,15 @@ describe('$uibModal', function() { | |||
| template:'<a href="#" id="tab-focus-link"><input type="text" id="tab-focus-input1"/><input type="text" id="tab-focus-input2"/>' + | |||
| '<button id="tab-focus-button">Open me!</button>' | |||
| }); | |||
| $rootScope.$digest(); | |||
There was a problem hiding this comment.
Is this extra $digest necessary? This seems a little suspicious to me.
There was a problem hiding this comment.
it's necessary to update focused element after modal opening
|
Verified here this looks good - thanks for the fix! |
Closes #5229