Focus issues

[pongells]

When open, the dialog is not taking focus. If the user presses Enter, the button that opened the dialog is re-triggered, thus opening another dialog. This behaviour can be seen in the demo, too.

[octavioamu]

Have the same problem here, the modal is not getting focus when opens. This a big issue about accessibility.

[voronianski]

@pongells @octavioamu I'll try to fix as soon as possible but any help and PRs are welcome!

/cc @grabbou

[octavioamu]

Thanks @voronianski I'm trying to figure out how to fix it, looking at bootstrap seems to have similiar issues (with angular) angular-ui/bootstrap#1696

There is also the boostrap function (I'm new with angularjs so don't know how to implement right) thats works different than in angular-ui https://github.com/twbs/bootstrap/blob/master/js/modal.js#L131

[voronianski]

@pongells @octavioamu guys, if you're still interested in this fix please test pull-request - #166

[srcn]

I just tested the PR and it works perfectly fine for me, no more multiple dialogs.

There is one thing I could suggest though. I have a custom select plugin (Selectize) as a first element in the dialog and auto focus causes Selectize to show its options list as soon as the dialog opens. It's not the case on native select inputs just the custom plugins.

As a test, I commented out this part:

var focusableElements = privateMethods.getFocusableElements($dialog);

if (focusableElements.length > 0) {
    focusableElements[0].focus();
    return;
}

It still focuses to the form correctly I guess because of the screen readers workaround that comes right after so maybe you can add another optional config for us to choose the focusable element so it will try to focus on that element rather than focusing the first one. Or maybe an option for disabling that portion of the auto focus code so it will only try to focus to the headers, p or span elements.

But anyways, thank you for your work and the fix.

[octavioamu]

Great!! Testing and looks good, when is going to be accepted ?

[bgse]

We've been working around this issue as shown below. Keep in mind though we're using custom templates, our own modified branch of ngDialog that has quite a bit diverged from the main branch, and have strict conventions on dialog layouts, so YMMV.

The basic idea though is that we first focus on the first button we find (a.btn, we use bootstrap, we always have dialog footers with button bar), and then see if the dialog has input fields and focus those instead.

This basically satisfies our UI convention that when a dialog comes up, the primary action button gets focus, or if the dialog has input fields, the first field gets focus.

$timeout(function () {
    $compile($dialog)(scope);

    var widthDiffs = $window.innerWidth - $body.prop('clientWidth');
    $body.addClass('ngdialog-open');
    var scrollBarWidth = widthDiffs - ($window.innerWidth - $body.prop('clientWidth'));
    if (scrollBarWidth > 0) {
        privateMethods.setBodyPadding(scrollBarWidth);
    }
    $body.append($dialog);

    /////////////////////////UGLY////////////////////////////////
    //hack for focus issue, set focus to first button we find...
    $('#ngdialog'+globalID).find('a.btn:first').focus();
    //then attempt to find a better place, like the first input
    $('#ngdialog'+globalID).find('input:first').focus();
    /////////////////////////////////////////////////////////////

    $rootScope.$broadcast('ngDialog.opened', $dialog);
});

[octavioamu]

I'm also using this version and the 'Ugly' thing is something about accessibility, is how the browser works and give us :focus and 'outline: something ' to style proposes, so the ugly thing is a 'must'.

[richardszalay]

@srcn Thanks for the feedback, though I'd recommend bringing the conversation to #166 as I only happened to stumble upon your comment.

What you describe is one of the edge cases I weighed against when enabling the accessibility features (specifically options.trapFocus) by default. You can disable it by passing in trapFocus: false with the options for the dialog, or you can .config() it into the default options or we can discuss disabling it by default.

Happy to discuss it further over in #166

[richardszalay]

@octavioamu My latest commit actually gets around this (not totally sure how happy I am with how it's done, though): if a non-focusable element (ie. h1-6,p,span,etc) is force-focused, the PR now forces .css('outline','none') to avoid any visual artifacts. Screen readers are still fine with it, though.

[richardszalay]

@bgse It looks like #166 will sort out this issue. It traps the focus within the dialog and automatically focuses something appropriate:

1. the first element with an "autofocus" attribute
2. the first focusable element in the dialog
3. the first content block, forcing a tabindex of -1 and styling out the outline (this mode also changes the "guessed" aria role from "dialog" to "alertdialog")

[jedwards1211]

From what I'm seeing in 0.12.0, the "autofocus" attribute works the first time I show a modal dialog, but not any time after that.

[pursual]

True this:

"From what I'm seeing in 0.12.0, the "autofocus" attribute works the first time I show a modal dialog, but not any time after that."

[richardszalay]

@jedwards1211 @pursual The autofocus support isn't in 0.12.0 as the pull request has't been merged yet. I assume it happens to work the first time in your tests because the browser picks it up the first time.

[slavafomin]

Hello!

I'm using latest version of ngModal (0.3.12). I have the same issue with focus. After dialog is opened, the button that opened it is still focused, so I can hit enter and open yet another dialog.

Is this going to be fixed? What are the workarounds?

Thank you!

[richardszalay]

Hi @slavafomin, try buliding against 7e5f67485f33cdd006dbc78a1f9586deacaab0ca or https://github.com/richardszalay/ngDialog#master, the pull request with focus improvement has been merged but no release has been created yet.

[Skinner927]

Current master seems to be working 👍

[MiladSadinam]

I am using ngDialog 0.4.0 and open a dialog with first input element having "autofocus". On firefox I get the expected behaviour. But on Chrome (Version 43.0.2357.65 m) not. There it seems the control gets the focus but immediately loses it again for some reason (as it has the "ng-touched" class). Anybody else has this issue?

[Ravindra042]

Facing the same problem with version v0.4.0. Can anybody explain me in simple steps, How to fix this.

[ghost]

the same for me. I have a lot of input elements, but when i open dialog date picker automatically opens and then closes immediately.

[Ravindra042]

Issue in detail -
ngDialog Popup looses the focus automatically in second and looks like background
Changes on css are not affecting the popup

[0000marcell]

is someone working on this issue ?

[faceleg]

@0000marcell not that I'm aware of, PR are very welcome :)

[PhiLhoSoft]

I still see the issue in the demo (and my own code, of course!) in latest Firefox and Chrome.
Support for autofocus is nice, although it might not work in all cases, since AngularJS content is dynamic in lot of cases, so it might not be practical to use this attribute. See angular/angular.js#10833 and linked issues for lot of discussion around this.

I think ideally it should work as suggested above: honor autofocus if found (simple dialogs like confirm), otherwise focus on first non-disabled button (like above, I don't want Selectize to drop down on dialog opening; but for this problem, I just set openOnFocus to false, actually), otherwise on the first non-disabled control. Or we should be able to specify explicitly which control will get the focus. Which can be done programmatically, anyway...

The main point is that the default behavior is wrong: the dialog should take the focus away from the control that spawned the dialog: we don't want Enter to open multiple instances of the dialog!
I wonder if we can set the focus on a hidden control, if nothing focusable is found...

[PhiLhoSoft]

For the record (as people will search this issue...), I planned to try and use https://github.com/medialize/ally.js to address that (never had time, alas). This project focused (sic) on this very problem (initially) although it has a broader goal. And it shows that's not a simple problem!

[faceleg]

Thanks for coming back with that note @PhiLhoSoft