-
Notifications
You must be signed in to change notification settings - Fork 6.8k
feat(overlay): add a utility for disabling body scroll #1971
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
3e145f1
3edc782
9f66f6d
b62c5b9
2d82339
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,144 @@ | ||
import {TestBed, async, inject} from '@angular/core/testing'; | ||
import {OverlayModule} from './overlay-directives'; | ||
import {DisableBodyScroll} from './disable-body-scroll'; | ||
|
||
|
||
describe('DisableBodyScroll', () => { | ||
let service: DisableBodyScroll; | ||
let startingWindowHeight = window.innerHeight; | ||
let forceScrollElement: HTMLElement = document.createElement('div'); | ||
|
||
forceScrollElement.style.height = '3000px'; | ||
forceScrollElement.style.width = '100px'; | ||
|
||
beforeEach(async(() => { | ||
TestBed.configureTestingModule({ | ||
imports: [OverlayModule.forRoot()], | ||
providers: [DisableBodyScroll] | ||
}); | ||
})); | ||
|
||
beforeEach(inject([DisableBodyScroll], (disableBodyScroll: DisableBodyScroll) => { | ||
service = disableBodyScroll; | ||
})); | ||
|
||
afterEach(() => { | ||
if (forceScrollElement.parentNode) { | ||
forceScrollElement.parentNode.removeChild(forceScrollElement); | ||
} | ||
|
||
service.deactivate(); | ||
}); | ||
|
||
it('should prevent scrolling', () => { | ||
document.body.appendChild(forceScrollElement); | ||
window.scrollTo(0, 100); | ||
|
||
// In the iOS simulator (BrowserStack & SauceLabs), adding the content to the | ||
// body causes karma's iframe for the test to stretch to fit that content once we attempt to | ||
// scroll the page. Setting width / height / maxWidth / maxHeight on the iframe does not | ||
// successfully constrain its size. As such, skip assertions in environments where the | ||
// window size has changed since the start of the test. | ||
if (window.innerHeight > startingWindowHeight) { | ||
return; | ||
} | ||
|
||
window.scrollTo(0, 100); | ||
|
||
service.activate(); | ||
|
||
window.scrollTo(0, 500); | ||
|
||
expect(window.pageYOffset).toBe(0); | ||
}); | ||
|
||
it('should toggle the isActive property', () => { | ||
document.body.appendChild(forceScrollElement); | ||
window.scrollTo(0, 100); | ||
|
||
// In the iOS simulator (BrowserStack & SauceLabs), adding the content to the | ||
// body causes karma's iframe for the test to stretch to fit that content once we attempt to | ||
// scroll the page. Setting width / height / maxWidth / maxHeight on the iframe does not | ||
// successfully constrain its size. As such, skip assertions in environments where the | ||
// window size has changed since the start of the test. | ||
if (window.innerHeight > startingWindowHeight) { | ||
return; | ||
} | ||
|
||
service.activate(); | ||
expect(service.isActive).toBe(true); | ||
|
||
service.deactivate(); | ||
expect(service.isActive).toBe(false); | ||
}); | ||
|
||
it('should not disable scrolling if the content is shorter than the viewport height', () => { | ||
service.activate(); | ||
expect(service.isActive).toBe(false); | ||
}); | ||
|
||
it('should add the proper inline styles to the <body> and <html> nodes', () => { | ||
document.body.appendChild(forceScrollElement); | ||
window.scrollTo(0, 500); | ||
|
||
// In the iOS simulator (BrowserStack & SauceLabs), adding the content to the | ||
// body causes karma's iframe for the test to stretch to fit that content once we attempt to | ||
// scroll the page. Setting width / height / maxWidth / maxHeight on the iframe does not | ||
// successfully constrain its size. As such, skip assertions in environments where the | ||
// window size has changed since the start of the test. | ||
if (window.innerHeight > startingWindowHeight) { | ||
return; | ||
} | ||
|
||
let bodyCSS = document.body.style; | ||
let htmlCSS = document.documentElement.style; | ||
|
||
service.activate(); | ||
|
||
expect(bodyCSS.position).toBe('fixed'); | ||
expect(bodyCSS.width).toBe('100%'); | ||
expect(bodyCSS.top).toBe('-500px'); | ||
expect(bodyCSS.maxWidth).toBeTruthy(); | ||
expect(htmlCSS.overflowY).toBe('scroll'); | ||
}); | ||
|
||
it('should revert any previously-set inline styles', () => { | ||
let bodyCSS = document.body.style; | ||
let htmlCSS = document.documentElement.style; | ||
|
||
document.body.appendChild(forceScrollElement); | ||
|
||
bodyCSS.position = 'static'; | ||
bodyCSS.width = '1000px'; | ||
htmlCSS.overflowY = 'hidden'; | ||
|
||
service.activate(); | ||
service.deactivate(); | ||
|
||
expect(bodyCSS.position).toBe('static'); | ||
expect(bodyCSS.width).toBe('1000px'); | ||
expect(htmlCSS.overflowY).toBe('hidden'); | ||
|
||
bodyCSS.cssText = ''; | ||
htmlCSS.cssText = ''; | ||
}); | ||
|
||
it('should restore the scroll position when enabling scrolling', () => { | ||
document.body.appendChild(forceScrollElement); | ||
window.scrollTo(0, 1000); | ||
|
||
// In the iOS simulator (BrowserStack & SauceLabs), adding the content to the | ||
// body causes karma's iframe for the test to stretch to fit that content once we attempt to | ||
// scroll the page. Setting width / height / maxWidth / maxHeight on the iframe does not | ||
// successfully constrain its size. As such, skip assertions in environments where the | ||
// window size has changed since the start of the test. | ||
if (window.innerHeight > startingWindowHeight) { | ||
return; | ||
} | ||
|
||
service.activate(); | ||
service.deactivate(); | ||
|
||
expect(window.pageYOffset).toBe(1000); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,74 @@ | ||
import {Injectable, Optional, SkipSelf} from '@angular/core'; | ||
import {ViewportRuler} from './position/viewport-ruler'; | ||
|
||
|
||
/** | ||
* Utilitity that allows for toggling scrolling of the viewport on/off. | ||
*/ | ||
@Injectable() | ||
export class DisableBodyScroll { | ||
private _bodyStyles: string = ''; | ||
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. Add descriptions for |
||
private _htmlStyles: string = ''; | ||
private _previousScrollPosition: number = 0; | ||
private _isActive: boolean = false; | ||
|
||
/** Whether scrolling is disabled. */ | ||
public get isActive(): boolean { | ||
return this._isActive; | ||
} | ||
|
||
constructor(private _viewportRuler: ViewportRuler) { } | ||
|
||
/** | ||
* Disables scrolling if it hasn't been disabled already and if the body is scrollable. | ||
*/ | ||
activate(): void { | ||
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. How able calling the methods |
||
let body = document.body; | ||
let bodyHeight = body.scrollHeight; | ||
let viewportHeight = this._viewportRuler.getViewportRect().height; | ||
|
||
if (!this.isActive && bodyHeight > viewportHeight) { | ||
let html = document.documentElement; | ||
let initialBodyWidth = body.clientWidth; | ||
|
||
this._htmlStyles = html.style.cssText || ''; | ||
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 caching the styles and setting them back, what do you think of just adding a removing a CSS class from the body where all of the rules are |
||
this._bodyStyles = body.style.cssText || ''; | ||
this._previousScrollPosition = this._viewportRuler.getViewportScrollPosition().top; | ||
|
||
body.style.position = 'fixed'; | ||
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. Are you about to talk at all about this approach vs. just setting 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. We could do that for the position and the width, but the |
||
body.style.width = '100%'; | ||
body.style.top = -this._previousScrollPosition + 'px'; | ||
html.style.overflowY = 'scroll'; | ||
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. Always setting Also, there can potentially be a horizontal scrollbar as well. |
||
|
||
// TODO(crisbeto): this avoids issues if the body has a margin, however it prevents the | ||
// body from adapting if the window is resized. check whether it's ok to reset the body | ||
// margin in the core styles. | ||
body.style.maxWidth = initialBodyWidth + 'px'; | ||
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'd prefer if we could avoid getting into the reset styles game. Can you go into more details about what the issue is with the default margin? 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. It has been a while since I wrote this, but IIRC the margin ended up throwing off some of the measurements. |
||
|
||
this._isActive = true; | ||
} | ||
} | ||
|
||
/** | ||
* Re-enables scrolling. | ||
*/ | ||
deactivate(): void { | ||
if (this.isActive) { | ||
document.body.style.cssText = this._bodyStyles; | ||
document.documentElement.style.cssText = this._htmlStyles; | ||
window.scroll(0, this._previousScrollPosition); | ||
this._isActive = false; | ||
} | ||
} | ||
} | ||
|
||
export function DISABLE_BODY_SCROLL_PROVIDER_FACTORY(parentDispatcher: DisableBodyScroll, | ||
viewportRuler: ViewportRuler) { | ||
return parentDispatcher || new DisableBodyScroll(viewportRuler); | ||
}; | ||
|
||
export const DISABLE_BODY_SCROLL_PROVIDER = { | ||
provide: DisableBodyScroll, | ||
deps: [[new Optional(), new SkipSelf(), DisableBodyScroll]], | ||
useFactory: DISABLE_BODY_SCROLL_PROVIDER_FACTORY | ||
}; |
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.
How about
PageScrollBlocker
?