Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[this._focusTrap is null] Or [Cannot read property 'destroy' of null] When calling close() #2372

Closed
akram1905 opened this issue May 9, 2018 · 3 comments

Comments

@akram1905
Copy link

akram1905 commented May 9, 2018

Bug description:

Sometimes, when calling close() error will be thrown.
Error: this._focusTrap is null

Link to minimally-working plunker that reproduces the issue:

https://stackblitz.com/edit/angular-hobhrq

Steps:

  • Make sure console is open.
  • Click on input, this will open datepicker dialog.
  • Click now on text above(outside), this will trigger an event in code, and will close datepicker. (Sometimes need to click twice)
  • Error will be thrown.

chrome: Cannot read property 'destroy' of null
FF: this._focusTrap is null

Version of Angular, ng-bootstrap, and Bootstrap:

Angular: 6.0.0

ng-bootstrap: 2.0.0

Bootstrap: 4.1.1

@akram1905 akram1905 changed the title TypeError: this._focusTrap is null When calling close() [TypeError: this._focusTrap is null] Or [Cannot read property 'destroy' of null] When calling close() May 9, 2018
@akram1905 akram1905 changed the title [TypeError: this._focusTrap is null] Or [Cannot read property 'destroy' of null] When calling close() [this._focusTrap is null] Or [Cannot read property 'destroy' of null] When calling close() May 9, 2018
@pkozlowski-opensource
Copy link
Member

@benouat could you try to dig into it to see what is going on?

@benouat
Copy link
Member

benouat commented May 11, 2018

@pkozlowski-opensource sure i'll have a look. This might be fixed in #2328 which implements another way of handling focustrap.

@benouat
Copy link
Member

benouat commented May 11, 2018

@akram1905 I suspect the focus you are using on the input to cause the problem (which I will fix)

The autofocus feature on the datepicker will by default (and we did not make it available via config) put the focus back on the element that was previously focused just before opening.

In your case the input, which listen on focus to open the datepicker again. Hence the crash, and also the fact that you need to click twice somehow.

On top of that, focustrap is using internally focus on any element outisde to gain focus again.

Be aware that for example, if you simply replace (focus)="dp.open()" by (click)="dp.open()" it simply doesn't occur anymore. (This is the way we are describing how to open a dartepicker from the datepicker icon in our example).

I would not recommend to open anything on focus, mainly for accessibility, it's better to announce that user can open something by triggering an action (like click) instead of having it to be open magically on navigation.

Nonetheless, I'll try to make this scenario work for sure ! Thanks for the report.

benouat pushed a commit to benouat/ng-bootstrap that referenced this issue May 22, 2018
maxokorokov added a commit to maxokorokov/ng-bootstrap that referenced this issue May 29, 2018
Refactor existing implementation of the focus trap: handle only inside clicks and tab navigation. It doesn't steal outside focus anymore

Fixes ng-bootstrap#2406
Fixes ng-bootstrap#2390
Fixes ng-bootstrap#2372 (one more time)
maxokorokov added a commit to maxokorokov/ng-bootstrap that referenced this issue May 29, 2018
Refactor existing implementation of the focus trap: handle only inside clicks and tab navigation. It doesn't steal outside focus anymore

Fixes ng-bootstrap#2406
Fixes ng-bootstrap#2390
Fixes ng-bootstrap#2372 (one more time)
maxokorokov added a commit to maxokorokov/ng-bootstrap that referenced this issue Jun 4, 2018
Refactor existing implementation of the focus trap: handle only inside clicks and tab navigation. It doesn't steal outside focus anymore

Fixes ng-bootstrap#2406
Fixes ng-bootstrap#2390
Fixes ng-bootstrap#2372 (one more time)
maxokorokov added a commit that referenced this issue Jun 4, 2018
Refactor existing implementation of the focus trap: handle only inside clicks and tab navigation. It doesn't steal outside focus anymore

Closes #2419
Fixes #2406
Fixes #2390
Fixes #2372 (one more time)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants