-
Notifications
You must be signed in to change notification settings - Fork 6.8k
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
fix(overlay): make overlay work in fullscreen mode #1653
Conversation
@jelbourn can you review my PR? It would be great to have this fix in next release. |
@quanterion working my way here- I need to set aside a chunk of time to look at the issue. |
Thanks for your patience on this one. I took a look at the issue and think a cleaner approach would be to introduce a new type of @Injectable()
export class FullscreenFriendlyOverlayContainer extends OverlayContainer {
protected _createContainer(): void {
super._createContainer();
this._addFullscreenChangeListener(() => this._adjustParentForFullscreenChange());
}
private _adjustParentForFullscreenChange(): void {
if (!this._containerElement) {
return;
}
let fullscreenElement = this._getFullscreenElement();
let parent = fullscreenElement || document.body;
parent.appendChild(this._containerElement);
}
private _addFullscreenChangeListener(fn: () => void) {
if (document.fullscreenEnabled) {
document.addEventListener('fullscreenchange', fn);
} else if (document.webkitFullscreenEnabled) {
document.addEventListener('webkitfullscreenchange', fn);
} else if ((document as any).mozFullScreenEnabled) {
document.addEventListener('mozfullscreenchange', fn);
} else if ((document as any).msFullscreenEnabled) {
document.addEventListener('MSFullscreenChange', fn);
}
}
private _getFullscreenElement(): Element {
return document.fullscreenElement ||
document.webkitFullscreenElement ||
(document as any).mozFullScreenElement ||
(document as any).msFullscreenElement ||
null;
}
} To use this container instead of the default, you would add a new provider to your app: providers: [
{provide: OverlayContainer, useClass: FullscreenFriendlyOverlayContainer},
] Doing it this way has a couple of advantages:
Would you be willing up update your PR to use this approach and add the appropriate tests / comments / docs? |
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
This allows all overlay-based components like dialog, menu, tooltip etc work in full screen mode https://developer.mozilla.org/en-US/docs/Web/API/Element/requestFullScreen
Implementation moves overlay container into fullscreen element and back to document.body as user navigates between fullscreen and non-fullscreen modes. Apps not using fullscreen mode are unaffected.
fixes #1628
PS: I added Fullscreen button near to RTL one to test fix across all components
PPS: I have no idea about tests for this fix, any advice?