-
Notifications
You must be signed in to change notification settings - Fork 6.8k
feat(popover-edit): allow tabbing from popup to next/previous cell #15793
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,74 @@ | ||
/** | ||
* @license | ||
* Copyright Google LLC All Rights Reserved. | ||
* | ||
* Use of this source code is governed by an MIT-style license that can be | ||
* found in the LICENSE file at https://angular.io/license | ||
*/ | ||
|
||
import {Inject, Injectable, NgZone} from '@angular/core'; | ||
import {DOCUMENT} from '@angular/common'; | ||
import {FocusTrap, InteractivityChecker} from '@angular/cdk/a11y'; | ||
import {Observable, Subject} from 'rxjs'; | ||
|
||
/** Value indicating whether focus left the target area before or after the enclosed elements. */ | ||
export const enum FocusEscapeNotifierDirection { | ||
START, | ||
END, | ||
} | ||
|
||
/** | ||
* Like FocusTrap, but rather than trapping focus within a dom region, notifies subscribers when | ||
* focus leaves the region. | ||
*/ | ||
export class FocusEscapeNotifier extends FocusTrap { | ||
private _escapeSubject = new Subject<FocusEscapeNotifierDirection>(); | ||
|
||
constructor( | ||
element: HTMLElement, | ||
checker: InteractivityChecker, | ||
ngZone: NgZone, | ||
document: Document) { | ||
super(element, checker, ngZone, document, true /* deferAnchors */); | ||
|
||
// The focus trap adds "anchors" at the beginning and end of a trapped region that redirect | ||
// focus. We override that redirect behavior here with simply emitting on a stream. | ||
this.startAnchorListener = () => { | ||
kseamon marked this conversation as resolved.
Show resolved
Hide resolved
|
||
this._escapeSubject.next(FocusEscapeNotifierDirection.START); | ||
return true; | ||
}; | ||
this.endAnchorListener = () => { | ||
this._escapeSubject.next(FocusEscapeNotifierDirection.END); | ||
return true; | ||
}; | ||
|
||
this.attachAnchors(); | ||
} | ||
|
||
escapes(): Observable<FocusEscapeNotifierDirection> { | ||
return this._escapeSubject.asObservable(); | ||
} | ||
} | ||
|
||
/** Factory that allows easy instantiation of focus escape notifiers. */ | ||
@Injectable({providedIn: 'root'}) | ||
export class FocusEscapeNotifierFactory { | ||
private _document: Document; | ||
|
||
constructor( | ||
private _checker: InteractivityChecker, | ||
private _ngZone: NgZone, | ||
@Inject(DOCUMENT) _document: any) { | ||
|
||
this._document = _document; | ||
} | ||
|
||
/** | ||
* Creates a focus escape notifier region around the given element. | ||
* @param element The element around which focus will be monitored. | ||
* @returns The created focus escape notifier instance. | ||
*/ | ||
create(element: HTMLElement): FocusEscapeNotifier { | ||
return new FocusEscapeNotifier(element, this._checker, this._ngZone, this._document); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -61,6 +61,7 @@ export class CdkEditControl<FormValue> implements OnDestroy, OnInit { | |
ngOnInit(): void { | ||
this.editRef.init(this.preservedFormValue); | ||
this.editRef.finalValue.subscribe(this.preservedFormValueChange); | ||
this.editRef.blurred.subscribe(() => this._handleBlur()); | ||
} | ||
|
||
ngOnDestroy(): void { | ||
|
@@ -97,7 +98,7 @@ export class CdkEditControl<FormValue> implements OnDestroy, OnInit { | |
switch (this.clickOutBehavior) { | ||
case 'submit': | ||
// Manually cause the form to submit before closing. | ||
this.elementRef.nativeElement!.dispatchEvent(new Event('submit')); | ||
this._triggerFormSubmit(); | ||
// Fall through | ||
case 'close': | ||
this.editRef.close(); | ||
|
@@ -106,6 +107,18 @@ export class CdkEditControl<FormValue> implements OnDestroy, OnInit { | |
break; | ||
} | ||
} | ||
|
||
/** Triggers submit on tab out if clickOutBehavior is 'submit'. */ | ||
private _handleBlur(): void { | ||
if (this.clickOutBehavior === 'submit') { | ||
// Manually cause the form to submit before closing. | ||
this._triggerFormSubmit(); | ||
} | ||
} | ||
|
||
private _triggerFormSubmit() { | ||
this.elementRef.nativeElement!.dispatchEvent(new Event('submit')); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Rather than "submitting" the form via the DOM, couldn't we call the submit handler ourselves? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You could alternatively call There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I do need the form itself to submit in case the user added any submit handlers. |
||
} | ||
} | ||
|
||
/** Reverts the form to its initial or previously submitted state on click. */ | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This could be in a follow-up change, but could we avoid using
any
here in favor of a generic?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That would be tricky - the type is generic in the lens (refers to the shape of the form data) but different edits in the same table could have different lenses and this must refer to all of them.
It’d be fine to use unknown here but I don’t know what TS versions we have to support.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can make it unknown in a follow-up. As of Angular v8 we support 3.4 as the minimum