feat: focustrap standalone service

[benouat]

PR introduces an internal service to setup a focus-trap feature to be added to any element dynamically created by a widget.

Usage context: any datepicker popup, popover, modals...
Usage example:

• provide the service in the widget module providers: [NgbFocusTrapFactory]
• inject it in the widget constructor: private _focusTrapFactory: NgbFocusTrapFactory
• call the create method specifying an element, it returns the newly created NgbFocusTrap instance

this._focusTrap = this._focusTrapFactory.create(this.elementRef.nativeElement)

• later on, when the dynamically created element is not needed, properly destroy the focustrap to release event listeners set on document: this._focusTrap.destory()

---

You can also decorate a node contained inside element with NgbAutofocus attribute, and specify a second boolean argument to the create() method:

<div>
  <span></span>
  <input type="text" ngbAutofocus />
  <div></div>
</div>

this._focusTrap = this._focusTrapFactory.create(this.elementRef.nativeElement, true)

this will automatically set the focus on the input when the focus gets initialised. On top it will remenber any previously focused element, and restore it when destroyed (feature needed to properly implement accessible modals)

[maxokorokov]

As discussed, two use cases need to be addressed:

• the case when focustrap is around something that is appended to body
• it might be better to call this._focusInitial() when the zone is stable

[maxokorokov]

shiftKey is not used

[benouat]

Removed !

[maxokorokov]

dispatchEvent is not used

[benouat]

Removed !

[maxokorokov]

This would be called synchronously, I'm not sure it will work for dynamically instantiated components.
I guess it would try to focus before the change detection is run, so should be checked

[benouat]

I am not sure it might be needed. Live tests seem to show that it is working in both datepicker and modal without that change.

Nonetheless, if we would ever need such a thing, I would rather prefer creating the focusTrap inside of an ngAfterViewInit to make sure that View is created and hence available to focus.

[benouat]

@maxokorokov I pushed a new commit, and added a small change to make sure it would work properly in modal.

The issue with modal is simple. Modal are appended to the body, so when you hit tab on the last focusable element of the modal, you basically have reached the end of the document. So we never go inside the focusin in focustrap.

Fix was simple, when focustrap is created I create a focusable anchor and append it to the body.

I added a test for that also.

[maxokorokov]

Hey, @benouat, thanks!

LGTM, just a couple of questions about NgbFocusTrap API after I did another pass. No big deal, just food for thought :)

Will merge tomorrow after you decide if it's worth fixing.

Feel free to open other PRs for Datepicker/Modal if you have time for it.

[maxokorokov]

It is neither used publicly nor in tests. Why have a public getter for it instead of a private function?

[benouat]

I used to have tests in one of the previous version. I will make it private

[maxokorokov]

Same question for autofocus setter. Do you see anybody calling it after constructing the service?

I mean doing service = factory.create(el) and then doing service.autofocus = true later, as opposed to service = factory.create(el, true) right away ?

[benouat]

You are right, it seems there is no added value to do that.
I will move the logic to constructor right away.

[maxokorokov]

Saucelabs failing as usual, will merge when green

[benouat]

Awesome ! Thanks

[ymeine]

You got a bit fast on merging this one - btw my PRs are forever pending - since it has some drawbacks.

First it doesn't handle the use case of coming from the outside of the viewport. Then it doesn't handle screen readers, where aria-hidden should be used. Finally, it re-implements a whole logic upon each tab/shift+tab keystroke while some other equivalent techniques don't. For people who tend to do remarks about micro-optimizations I find this one a bit borderline.

Also, since usually the Angular way is pushed, I thought that it would have been a good use case for a directive.

Not to mention some other use cases not handled such as elements hidden because of a parent which is not displayed.

So please don't overlook team work, I've worked on this task for quite a while now (doing researches to know what would be best for this product) so I have some knowledge about it, and also have an implementation already done before which handles those mentioned use cases.