Add tests for sticky-header by sllethe · Pull Request #6074 · angular/components

sllethe · 2017-07-26T22:35:10Z

Added tests for sticky-header. 👍
Please take another look.

@Injectable

# This is the 1st commit message: # This is a combination of 11 commits. # This is the 1st commit message: add lib files for sticky-header add chose parent add support to 'optional 'cdkStickyRegion' input ' add app-demo for sticky-header fix bugs and deleted unused tag id in HTML files modify fix some code according to PR review comments change some format to pass TSlint check add '_' before private elements delete @Injectable for StickyHeaderDirective. Because we do not need @Injectable refine code encapsulate 'set style for element' change @input() Delete 'Observable.fromEvent(this.upperScrollableContainer, 'scroll')' add const STICK_START_CLASS and STICK_END_CLASS Add doc for [cdkStickyRegion] and 'unstuckElement()'. Delete 'detach()' function, add its content into 'ngOnDestroy()'. change 'MdStickyHeaderModule' to 'CdkStickyHeaderModule'; encapsulate reset css style operation for sticky header. delete unnecessary gloable variables delete global variable '_width' Add doc for 'sticker()' function. explained how it works. add more doc for 'sticker()', explaining 'isStuck' flag 2 space for indent # This is the commit message angular#2: fix # This is the commit message angular#3: delete sticky-header demo part from this branch # This is the commit message angular#4: revert firebase file # This is the commit message angular#5: change code according to comments in PR # This is the commit message angular#6: revert firbaserc # This is the commit message angular#7: revert demo-app.ts # This is the commit message angular#8: revert routes.ts # This is the commit message angular#9: revert demo-app-module.ts # This is the commit message angular#10: change # This is the commit message angular#11: fix the problem of : 'this.stickyParent' might be 'null' # This is the commit message angular#2: change doc # This is the commit message angular#3: Change the constructor of 'cdkStickyRegion' to 'constructor(public readonly _elementRef: ElementRef) { }' # This is the commit message angular#4: Added prefix 'mat-' for CSS class # This is the commit message angular#5: Delete 'public' before variables # This is the commit message angular#6: Object.assign isn't supported in IE11; use extendObject from src/lib/core/util. # This is the commit message angular#7: IE11 will have trouble with `translate3d(0, 0, 0);', change to `translate3d(0px, 0px, 0px);' # This is the commit message angular#8: Added docs for all variables # This is the commit message angular#9: extract 'generate CSS style'

@Injectable

# This is the 1st commit message: add lib files for sticky-header add chose parent add support to 'optional 'cdkStickyRegion' input ' add app-demo for sticky-header fix bugs and deleted unused tag id in HTML files modify fix some code according to PR review comments change some format to pass TSlint check add '_' before private elements delete @Injectable for StickyHeaderDirective. Because we do not need @Injectable refine code encapsulate 'set style for element' change @input() Delete 'Observable.fromEvent(this.upperScrollableContainer, 'scroll')' add const STICK_START_CLASS and STICK_END_CLASS Add doc for [cdkStickyRegion] and 'unstuckElement()'. Delete 'detach()' function, add its content into 'ngOnDestroy()'. change 'MdStickyHeaderModule' to 'CdkStickyHeaderModule'; encapsulate reset css style operation for sticky header. delete unnecessary gloable variables delete global variable '_width' Add doc for 'sticker()' function. explained how it works. add more doc for 'sticker()', explaining 'isStuck' flag 2 space for indent fix delete sticky-header demo part from this branch revert firebase file change code according to comments in PR revert firbaserc revert demo-app.ts revert routes.ts revert demo-app-module.ts change fix the problem of : 'this.stickyParent' might be 'null' change doc Change the constructor of 'cdkStickyRegion' to 'constructor(public readonly _elementRef: ElementRef) { }' Added prefix 'mat-' for CSS class Delete 'public' before variables Object.assign isn't supported in IE11; use extendObject from src/lib/core/util. IE11 will have trouble with `translate3d(0, 0, 0);', change to `translate3d(0px, 0px, 0px);' Added docs for all variables extract 'generate CSS style' created a generateStyleCSS() function, let it be responsible for generating all those CSS styles. reformat add debounce to solve 'getBoundingClientRect() cause slow down' problem. add position:sticky and check whether browser support it. If not , use the naive implementation removed unused import Removed unused 'scrollableRegion' and 'parentRegion' removed commented lines default public Add comments about why setting style top and position for iPhone and not IE. And extract detectBrowser() as a new function format consider all circumstances of browser. use "===" instead of '==' make 'navigator.userAgent.toLocaleLowerCase()' a local variable optimize Added comments on const 'STICK_START_CLASS' and 'STICK_END_CLASS'. change their content to cdk-sticky-header-start and cdk-sticky-header-end Added comments for STICK_START_CLASS and STICK_END_CLASS. Changed the format of one-line JsDoc unsubscribe sbscriptions onDestory Use what modernizr does on compatibility instead of get the browser version directly. add 'padding' and 'stickyRegionHeight' variables to avoid calling 'getComputedStyle()' too many times (which is expensive). move docs above @directive removed the underscore in'_element: ElementRef', expand 'reg' to 'region' use 'if (this.isIE)' instead of 'if(this.isIE === true)' added more newlines between params in 'generateCssStyle()' function to make it easier to understand. Added reference link to Modernizer in docs of getSupportList() Deleted "_supportList" variable renamed 'isIE' to 'isStickyPositionSupported', and removed extra space before Observable Set debounce time as a const variable Added docs for 'const DEBOUNCE_TIME: number = 5;' Changed ' if(this.stickyParent == null)' to ' if(!this.stickyParent)' Removed the @PARAM and @returns and make sure the types are correct in the function signature in 'generateCssStyle(...)' function Added docs for `isStickyPositionSupported` variable changed '+=' to '=' of 'stickyText' in getSupportList() function nit added " " between 'if' and '(' nit Added comments deleted unused import change comments optimize comments deleted unnecessary global variables(padding and stickyRegionHeight) Added check whether we are on browser Array<string> to string[] test? try to reopen the old PR fix after rebase revert list.ts test test 222 revert demo revert list.ts second time Move code to 'src/cdk' revert 'move code to 'src/cdk'' , it should be done in a new PR revert avoid calling 'getComputedStyle()' too many times. rename as sticky-header.ts # This is the commit message angular#2: imported PlatformModule # This is the commit message angular#3: Add blank lines between these top-level symbol # This is the commit message angular#4: make '_isStickyPositionSupported' private # This is the commit message angular#5: Changed the originalCSS to private and use '{} as CSSStyleDeclaration' instead of ''any.

@Injectable

# This is the 1st commit message: # This is a combination of 10 commits. # This is the 1st commit message: add lib files for sticky-header add chose parent add support to 'optional 'cdkStickyRegion' input ' add app-demo for sticky-header fix bugs and deleted unused tag id in HTML files modify fix some code according to PR review comments change some format to pass TSlint check add '_' before private elements delete @Injectable for StickyHeaderDirective. Because we do not need @Injectable refine code encapsulate 'set style for element' change @input() Delete 'Observable.fromEvent(this.upperScrollableContainer, 'scroll')' add const STICK_START_CLASS and STICK_END_CLASS Add doc for [cdkStickyRegion] and 'unstuckElement()'. Delete 'detach()' function, add its content into 'ngOnDestroy()'. change 'MdStickyHeaderModule' to 'CdkStickyHeaderModule'; encapsulate reset css style operation for sticky header. delete unnecessary gloable variables delete global variable '_width' Add doc for 'sticker()' function. explained how it works. add more doc for 'sticker()', explaining 'isStuck' flag 2 space for indent fix delete sticky-header demo part from this branch revert firebase file change code according to comments in PR revert firbaserc revert demo-app.ts revert routes.ts revert demo-app-module.ts change fix the problem of : 'this.stickyParent' might be 'null' change doc Change the constructor of 'cdkStickyRegion' to 'constructor(public readonly _elementRef: ElementRef) { }' Added prefix 'mat-' for CSS class Delete 'public' before variables Object.assign isn't supported in IE11; use extendObject from src/lib/core/util. IE11 will have trouble with `translate3d(0, 0, 0);', change to `translate3d(0px, 0px, 0px);' Added docs for all variables extract 'generate CSS style' created a generateStyleCSS() function, let it be responsible for generating all those CSS styles. reformat add debounce to solve 'getBoundingClientRect() cause slow down' problem. add position:sticky and check whether browser support it. If not , use the naive implementation removed unused import Removed unused 'scrollableRegion' and 'parentRegion' removed commented lines default public Add comments about why setting style top and position for iPhone and not IE. And extract detectBrowser() as a new function format consider all circumstances of browser. use "===" instead of '==' make 'navigator.userAgent.toLocaleLowerCase()' a local variable optimize Added comments on const 'STICK_START_CLASS' and 'STICK_END_CLASS'. change their content to cdk-sticky-header-start and cdk-sticky-header-end Added comments for STICK_START_CLASS and STICK_END_CLASS. Changed the format of one-line JsDoc unsubscribe sbscriptions onDestory Use what modernizr does on compatibility instead of get the browser version directly. add 'padding' and 'stickyRegionHeight' variables to avoid calling 'getComputedStyle()' too many times (which is expensive). move docs above @directive removed the underscore in'_element: ElementRef', expand 'reg' to 'region' use 'if (this.isIE)' instead of 'if(this.isIE === true)' added more newlines between params in 'generateCssStyle()' function to make it easier to understand. Added reference link to Modernizer in docs of getSupportList() Deleted "_supportList" variable renamed 'isIE' to 'isStickyPositionSupported', and removed extra space before Observable Set debounce time as a const variable Added docs for 'const DEBOUNCE_TIME: number = 5;' Changed ' if(this.stickyParent == null)' to ' if(!this.stickyParent)' Removed the @PARAM and @returns and make sure the types are correct in the function signature in 'generateCssStyle(...)' function Added docs for `isStickyPositionSupported` variable changed '+=' to '=' of 'stickyText' in getSupportList() function nit added " " between 'if' and '(' nit Added comments deleted unused import change comments optimize comments deleted unnecessary global variables(padding and stickyRegionHeight) Added check whether we are on browser Array<string> to string[] test? try to reopen the old PR fix after rebase revert list.ts test test 222 revert demo revert list.ts second time Move code to 'src/cdk' revert 'move code to 'src/cdk'' , it should be done in a new PR revert avoid calling 'getComputedStyle()' too many times. rename as sticky-header.ts imported PlatformModule Add blank lines between these top-level symbol make '_isStickyPositionSupported' private Changed the originalCSS to private and use '{} as CSSStyleDeclaration' instead of ''any. Rename '_containerStart' to '_stickyRegionTop' # This is the commit message angular#2: rename # This is the commit message angular#3: optimize discription for '_stickyRegionBottomThreshold' # This is the commit message angular#4: private _originalStyles = { position: '', top: '', right: '', left: '', bottom: '', width: '', zIndex: ''}; # This is the commit message angular#5: Deleted 'generateCssStyle()' and 'getCssNumber()' function # This is the commit message angular#6: Deleted 'getCssValue()' function # This is the commit message angular#7: fix CSSStyleDeclaration # This is the commit message angular#8: change sticky width to 'this.upperScrollableContainer.clientWidth' # This is the commit message angular#9: fix # This is the commit message angular#10: nit # This is the commit message angular#2: Added the 'isPositionStickySupported() ' function to src/cdk/platform/features.ts. Consume that function in this component and just always use both the webkit and unprefixed styles. # This is the commit message angular#3: nit # This is the commit message angular#4: nit # This is the commit message angular#5: update doc 'Debounce time in milliseconds for events that affect the sticky positioning (e.g. scroll, resize, touch move). Set as 5 milliseconds which is the highest delay that doesn't drastically affect the positioning adversely.' # This is the commit message angular#6: changed the doc to '/** z-index to be applied to the sticky header (default is 10). */' # This is the commit message angular#7: fix tslint error # This is the commit message angular#8: for comment 'Can you evaluate each method to make sure their accessor privacy is right? E.g. see which functions need to be public, private, static, etc' # This is the commit message angular#9: Deleted variable 'elemHeight' # This is the commit message angular#10: Chaned to 'if (!this.stickyParent)' # This is the commit message angular#11: Simplified Docs for 'sticker()'. # This is the commit message angular#12: set 'defineRestriction()' function to private # This is the commit message angular#13: use 'RxChain' # This is the commit message angular#14: deleted unused 'tableModule' in modules.ts # This is the commit message angular#15: rename to '_isPositionStickySupported' # This is the commit message angular#16: Use // for comments, /* */ for docs

@Injectable

add chose parent add support to 'optional 'cdkStickyRegion' input ' add app-demo for sticky-header fix bugs and deleted unused tag id in HTML files modify fix some code according to PR review comments change some format to pass TSlint check add '_' before private elements delete @Injectable for StickyHeaderDirective. Because we do not need @Injectable refine code encapsulate 'set style for element' change @input() Delete 'Observable.fromEvent(this.upperScrollableContainer, 'scroll')' add const STICK_START_CLASS and STICK_END_CLASS Add doc for [cdkStickyRegion] and 'unstuckElement()'. Delete 'detach()' function, add its content into 'ngOnDestroy()'. change 'MdStickyHeaderModule' to 'CdkStickyHeaderModule'; encapsulate reset css style operation for sticky header. delete unnecessary gloable variables delete global variable '_width' Add doc for 'sticker()' function. explained how it works. add more doc for 'sticker()', explaining 'isStuck' flag 2 space for indent fix delete sticky-header demo part from this branch revert firebase file change code according to comments in PR revert firbaserc revert demo-app.ts revert routes.ts revert demo-app-module.ts change fix the problem of : 'this.stickyParent' might be 'null' change doc Change the constructor of 'cdkStickyRegion' to 'constructor(public readonly _elementRef: ElementRef) { }' Added prefix 'mat-' for CSS class Delete 'public' before variables Object.assign isn't supported in IE11; use extendObject from src/lib/core/util. IE11 will have trouble with `translate3d(0, 0, 0);', change to `translate3d(0px, 0px, 0px);' Added docs for all variables extract 'generate CSS style' created a generateStyleCSS() function, let it be responsible for generating all those CSS styles. reformat add debounce to solve 'getBoundingClientRect() cause slow down' problem. add position:sticky and check whether browser support it. If not , use the naive implementation removed unused import Removed unused 'scrollableRegion' and 'parentRegion' removed commented lines default public Add comments about why setting style top and position for iPhone and not IE. And extract detectBrowser() as a new function format consider all circumstances of browser. use "===" instead of '==' make 'navigator.userAgent.toLocaleLowerCase()' a local variable optimize Added comments on const 'STICK_START_CLASS' and 'STICK_END_CLASS'. change their content to cdk-sticky-header-start and cdk-sticky-header-end Added comments for STICK_START_CLASS and STICK_END_CLASS. Changed the format of one-line JsDoc unsubscribe sbscriptions onDestory Use what modernizr does on compatibility instead of get the browser version directly. add 'padding' and 'stickyRegionHeight' variables to avoid calling 'getComputedStyle()' too many times (which is expensive). move docs above @directive removed the underscore in'_element: ElementRef', expand 'reg' to 'region' use 'if (this.isIE)' instead of 'if(this.isIE === true)' added more newlines between params in 'generateCssStyle()' function to make it easier to understand. Added reference link to Modernizer in docs of getSupportList() Deleted "_supportList" variable renamed 'isIE' to 'isStickyPositionSupported', and removed extra space before Observable Set debounce time as a const variable Added docs for 'const DEBOUNCE_TIME: number = 5;' Changed ' if(this.stickyParent == null)' to ' if(!this.stickyParent)' Removed the @PARAM and @returns and make sure the types are correct in the function signature in 'generateCssStyle(...)' function Added docs for `isStickyPositionSupported` variable changed '+=' to '=' of 'stickyText' in getSupportList() function nit added " " between 'if' and '(' nit Added comments deleted unused import change comments optimize comments deleted unnecessary global variables(padding and stickyRegionHeight) Added check whether we are on browser Array<string> to string[] test? try to reopen the old PR fix after rebase revert list.ts test test 222 revert demo revert list.ts second time Move code to 'src/cdk' revert 'move code to 'src/cdk'' , it should be done in a new PR revert avoid calling 'getComputedStyle()' too many times. rename as sticky-header.ts imported PlatformModule Add blank lines between these top-level symbol make '_isStickyPositionSupported' private Changed the originalCSS to private and use '{} as CSSStyleDeclaration' instead of ''any. Rename '_containerStart' to '_stickyRegionTop' rename optimize discription for '_stickyRegionBottomThreshold' private _originalStyles = { position: '', top: '', right: '', left: '', bottom: '', width: '', zIndex: ''}; Deleted 'generateCssStyle()' and 'getCssNumber()' function Deleted 'getCssValue()' function fix CSSStyleDeclaration change sticky width to 'this.upperScrollableContainer.clientWidth' fix nit Added the 'isPositionStickySupported() ' function to src/cdk/platform/features.ts. Consume that function in this component and just always use both the webkit and unprefixed styles. nit nit update doc 'Debounce time in milliseconds for events that affect the sticky positioning (e.g. scroll, resize, touch move). Set as 5 milliseconds which is the highest delay that doesn't drastically affect the positioning adversely.' changed the doc to '/** z-index to be applied to the sticky header (default is 10). */' fix tslint error for comment 'Can you evaluate each method to make sure their accessor privacy is right? E.g. see which functions need to be public, private, static, etc' Deleted variable 'elemHeight' Chaned to 'if (!this.stickyParent)' Simplified Docs for 'sticker()'. set 'defineRestriction()' function to private use 'RxChain' deleted unused 'tableModule' in modules.ts rename to '_isPositionStickySupported' Use // for comments, /* */ for docs @angular/cdk rename : values -> headerStyles Move closing brace to the next line optimized: [this._onScrollSubscription, this._onScrollSubscription, this._onResizeSubscription] .forEach(s => s && s.unsubscribe()); You should be able to do just '0' instead of '0px' Format like TODO(sllethe): ... private _attachEventListeners? Add a description like "Add listeners for events that affect sticky positioning." optimize doc Rename 'defineRestrictions' to '_measureStickyRegionBounds' rename: private _resetElementStyles let stuckRight: any = this.upperScrollableContainer.getBoundingClientRect().right; chaned 'any' to 'number' nit change doc '/** * Unsticks the header so that it goes back to scrolling normally. * * This should be called when the element reaches the bottom of its cdkStickyRegion so that it * smoothly scrolls out of view as the next sticky-header moves in. */' _unstuckElement -> _unstickElement rename 'sticker()' to '_applyStickyPositionStyles()' rename 'defineRestrictionsAndStick()' to '_updateStickyPositioning()'

deleted

tinayuangao · 2017-07-26T23:04:45Z

+/** Create a factory for sticky-positioning check to make code more testable */
+export const STICKY_HEADER_SUPPORT_STRATEGY_PROVIDER = {
+ provide: STICKY_HEADER_SUPPORT_STRATEGY,
+ deps: [],


Can remove deps if it's empty

👍 Removed deps

tinayuangao · 2017-07-26T23:06:17Z

+export const STICKY_HEADER_SUPPORT_STRATEGY_PROVIDER = {
+ provide: STICKY_HEADER_SUPPORT_STRATEGY,
+ deps: [],
+ useFactory: STICKY_HEADER_SUPPORT_STRATEGY_FACTORY


You can use useFactory: isPositionStickySupported, and move this part to index.ts:
providers: [{provide: STICKY_HEADER_SUPPORT_STRATEGY, useFactory: isPositionStickySupported}]

👍
Found that it doesn't work in index.ts. So put it back to sticky-header.ts. Changed it to :
export const STICKY_HEADER_SUPPORT_STRATEGY = new InjectionToken('sticky-header-support-strategy'); /** Create a factory for sticky-positioning check to make code more testable */ export const STICKY_HEADER_SUPPORT_STRATEGY_PROVIDER: Provider = { provide: STICKY_HEADER_SUPPORT_STRATEGY, useFactory: isPositionStickySupported };` ```

tinayuangao · 2017-07-26T23:07:18Z

 @Optional() public parentRegion: CdkStickyRegion,
- platform: Platform) {
+ platform: Platform,
+ @Inject(STICKY_HEADER_SUPPORT_STRATEGY) public _stickyPositionSupportCheck) {


Can use _isPositionStickySupported here as variable name and then you can remove line 155

👍 Changed it to _isPositionStickySupported and removed line155

tinayuangao · 2017-07-26T23:09:18Z

+ imports: [ OverlayModule, PlatformModule, StickyHeaderModule ],
+ declarations: [StickyHeaderTest],
+ providers: [
+ {provide: STICKY_HEADER_SUPPORT_STRATEGY_PROVIDER, useFactory: mockStickyHeaderCheckFail()},


You can use
providers: [ {provide: STICKY_HEADER_SUPPORT_STRATEGY_PROVIDER, useValue: false}]
and remove mockStickyHeaderCheckFail

tinayuangao · 2017-07-26T23:10:01Z

+ });
+
+ it('should be able to find stickyParent when sticky positioning is not supported', () => {
+ fixture.detectChanges();


nit: can move fixture.detectChanges(); after line 38 so you don't have to call it in every test

👍 Deleted fixture.detectChanges() after line 38

tinayuangao · 2017-07-26T23:10:33Z

+ imports: [ OverlayModule, PlatformModule, StickyHeaderModule ],
+ declarations: [StickyHeaderTest],
+ providers: [
+ {provide: STICKY_HEADER_SUPPORT_STRATEGY_PROVIDER,


same here, use useValue: true

👍 Changed it to useValue: true

removed 'deps' in providers

put provider in index

Removed unused 'fixture.detectChanges()' in test

sllethe

Comments addressed 👍
Please take another look

sllethe · 2017-07-27T00:03:10Z

+/** Create a factory for sticky-positioning check to make code more testable */
+export const STICKY_HEADER_SUPPORT_STRATEGY_PROVIDER = {
+ provide: STICKY_HEADER_SUPPORT_STRATEGY,
+ deps: [],


👍 Removed deps

sllethe · 2017-07-27T16:50:07Z

+export const STICKY_HEADER_SUPPORT_STRATEGY_PROVIDER = {
+ provide: STICKY_HEADER_SUPPORT_STRATEGY,
+ deps: [],
+ useFactory: STICKY_HEADER_SUPPORT_STRATEGY_FACTORY


👍
Found that it doesn't work in index.ts. So put it back to sticky-header.ts. Changed it to :
export const STICKY_HEADER_SUPPORT_STRATEGY = new InjectionToken('sticky-header-support-strategy'); /** Create a factory for sticky-positioning check to make code more testable */ export const STICKY_HEADER_SUPPORT_STRATEGY_PROVIDER: Provider = { provide: STICKY_HEADER_SUPPORT_STRATEGY, useFactory: isPositionStickySupported };` ```

sllethe · 2017-07-27T16:52:06Z

 @Optional() public parentRegion: CdkStickyRegion,
- platform: Platform) {
+ platform: Platform,
+ @Inject(STICKY_HEADER_SUPPORT_STRATEGY) public _stickyPositionSupportCheck) {


👍 Changed it to _isPositionStickySupported and removed line155

sllethe · 2017-07-27T16:55:31Z

+ imports: [ OverlayModule, PlatformModule, StickyHeaderModule ],
+ declarations: [StickyHeaderTest],
+ providers: [
+ {provide: STICKY_HEADER_SUPPORT_STRATEGY_PROVIDER,


👍 Changed it to useValue: true

sllethe · 2017-07-27T17:11:54Z

+ });
+
+ it('should be able to find stickyParent when sticky positioning is not supported', () => {
+ fixture.detectChanges();


👍 Deleted fixture.detectChanges() after line 38

sllethe · 2017-07-27T17:12:03Z

+ imports: [ OverlayModule, PlatformModule, StickyHeaderModule ],
+ declarations: [StickyHeaderTest],
+ providers: [
+ {provide: STICKY_HEADER_SUPPORT_STRATEGY_PROVIDER, useFactory: mockStickyHeaderCheckFail()},


tinayuangao · 2017-07-27T18:28:33Z

+ });
+
+ it('should be able to find stickyParent when sticky positioning is not supported', () => {
+ expect(stickyElement.nativeElement.stickyParent).not.toBe(null);


Is it stickyHeaderDir.stickyParent?

Yes 👍
Changed to stickyHeaderDir.stickyParent

tinayuangao · 2017-07-27T18:29:16Z

+ scrollableElement = fixture.debugElement.query(By.directive(Scrollable));
+ });
+
+ it('should be able to find stickyParent when sticky positioning is not supported', () => {


You can remove when sticky positioning is not supported since it's in describe

👍 Removed

tinayuangao · 2017-07-27T18:29:31Z

+ });
+
+ it('should be able to find scrollableContainer when sticky positioning is not supported', () => {
+ expect(stickyElement.nativeElement.upperScrollableContainer).not.toBe(null);


👍 Changed to stickyHeaderDir.upperScrollableContainer

tinayuangao · 2017-07-27T18:31:51Z

+ useFactory: isPositionStickySupported
+};
+
+// /** Create a factory for sticky-positioning check to make code more testable */


👍 Removed

You can remove when sticky positioning is not supported since it's in describe

stickyHeaderDir.stickyParent

remove commented code

sllethe · 2017-07-27T18:59:51Z

Comments Addressed 👍
Please take another look

jelbourn · 2017-07-29T00:27:02Z

+ testComponent = fixture.debugElement.componentInstance;
+ stickyElement = fixture.debugElement.query(By.directive(CdkStickyHeader));
+ stickyParentElement = fixture.debugElement.query(By.directive(CdkStickyRegion));
+ stickyHeaderDir = stickyElement.injector.get<CdkStickyHeader>(CdkStickyHeader);


Just stickyHeader is a good name. Dir makes me think it is short for "direction"

👍 Renamed stickyHeaderDir to stickyHeader

jelbourn · 2017-07-29T00:28:42Z

+ -moz-appearance: none;
+ height: 300px;
+ overflow: auto;">
+ <p>test test test</p>


Rather than hard-coding a bunch of divs, you could use ngFor to expand the content

👍 Changed to use 'ngFor' to expand content instead of typing tons of ''

jelbourn · 2017-07-29T00:29:42Z

+
+@Component({
+ template: `
+ <div cdk-scrollable style="text-align: center;


Add a comment that explains what these inline styles are for? Any reason you can't set them via the styles configuration for the component?

👍 Added comments on why we need to apply these styles. And put set styles via the styles configuration for the component.

jelbourn · 2017-07-29T00:31:18Z


+export const STICKY_HEADER_SUPPORT_STRATEGY = new InjectionToken('sticky-header-support-strategy');
+
+/** Create a factory for sticky-positioning check to make code more testable */


Should probably mark this as @docs-private

👍 Added /** @docs-private */

Changed to use 'ngFor' to expand content instead of typing tons of ''

Add a comment that explains what these inline styles are for? Any reason you can't set them via the styles configuration for the component?

Added explainations on styles

Added /** @docs-private */

sllethe

Comments Addressed 👍
Please take another look

sllethe · 2017-07-31T18:00:03Z

+ testComponent = fixture.debugElement.componentInstance;
+ stickyElement = fixture.debugElement.query(By.directive(CdkStickyHeader));
+ stickyParentElement = fixture.debugElement.query(By.directive(CdkStickyRegion));
+ stickyHeaderDir = stickyElement.injector.get<CdkStickyHeader>(CdkStickyHeader);


👍 Renamed stickyHeaderDir to stickyHeader

sllethe · 2017-07-31T18:34:29Z

+ -moz-appearance: none;
+ height: 300px;
+ overflow: auto;">
+ <p>test test test</p>


👍 Changed to use 'ngFor' to expand content instead of typing tons of ''

sllethe · 2017-07-31T18:45:58Z

+
+@Component({
+ template: `
+ <div cdk-scrollable style="text-align: center;


👍 Added comments on why we need to apply these styles. And put set styles via the styles configuration for the component.

sllethe · 2017-07-31T18:48:31Z


+export const STICKY_HEADER_SUPPORT_STRATEGY = new InjectionToken('sticky-header-support-strategy');
+
+/** Create a factory for sticky-positioning check to make code more testable */


👍 Added /** @docs-private */

Added comments to explaining why tick(100) is needed. Changed 'tick(0)' to 'tick()'

sllethe

Comments Addressed 👍
Please take another look

sllethe · 2017-07-31T19:05:06Z

+ let scrollableContainerTop = stickyHeader.upperScrollableContainer
+ .getBoundingClientRect().top;
+ expect(stickyHeader.element.getBoundingClientRect().top).not.toBe(scrollableContainerTop);
+ tick(0);


👍 Added comments to explaining why tick(100) is needed. Changed 'tick(0)' to 'tick()'
// Use tick(100) to tick forward to let stickyHeader's _applyStickyPositionStyles() // function finished

sllethe · 2017-07-31T19:09:07Z

+ it('should find sticky positioning is applied', () => {
+ let position = window.getComputedStyle(stickyHeader.element).position;
+ expect(position).not.toBe(null);
+ if (position != null) {


Because in expect(/sticky/i.test(position)).toBe(true);, the test() function's parameter can only be string. And the type of position is string | null, which will cause an error. S we need to add the '!= null' first.

jelbourn · 2017-07-31T21:40:14Z

+ // Scroll the scrollableContainer up to stick
+ fixture.componentInstance.scrollDown();
+ // Use tick(100) to tick forward to let stickyHeader's _applyStickyPositionStyles()
+ // function finished


Nothing in _applyStickyPositionStyles uses a setTimeout, so it's not clear why you need the 100 instead of just saying tick()

Because after we calling fixture.componentInstance.scrollDown();, the scrollDown() function dispatches a fake scroll event, which causes the _applyStickyPositionStyles being called. And When it finds that the header needs to be stuck, it will reassign value to the header , so the header will be repainted. And we need to wait till animation has finished.
Changed the comments to //wait till animation has finished'

jelbourn · 2017-07-31T21:42:07Z

+import {async, ComponentFixture, TestBed, fakeAsync, tick} from '@angular/core/testing';
+import {Component, DebugElement, ViewChild} from '@angular/core';
+import {StickyHeaderModule, CdkStickyRegion, CdkStickyHeader,
+ STICKY_HEADER_SUPPORT_STRATEGY} from './index';


If you run the "optimize imports" command in webstorm it will format these imports for you.

jelbourn · 2017-07-31T21:43:44Z

+ it('should find sticky positioning is applied', () => {
+ let position = window.getComputedStyle(stickyHeader.element).position;
+ expect(position).not.toBe(null);
+ if (position != null) {


You can tell typescript that you know something won't be null by adding an ! to the end of an expression:
expect(/sticky/i.test(position!)).toBe(true);
Then you don't need the if block

jelbourn · 2017-07-31T21:44:21Z

+ -webkit-appearance: none;
+ -moz-appearance: none;
+ height: 300px;
+ overflow: auto;


-2 indent in the css (the properties are indented as +4 right now, they should be +2)

👍 Changed the indent to +2

nit

explained why need 'tick(100)'

expect(/sticky/i.test(position!)).toBe(true);

sllethe

Comments Addressed 👍
Please take another look

sllethe · 2017-07-31T22:57:59Z

+import {async, ComponentFixture, TestBed, fakeAsync, tick} from '@angular/core/testing';
+import {Component, DebugElement, ViewChild} from '@angular/core';
+import {StickyHeaderModule, CdkStickyRegion, CdkStickyHeader,
+ STICKY_HEADER_SUPPORT_STRATEGY} from './index';


sllethe · 2017-07-31T23:10:54Z

+ // Scroll the scrollableContainer up to stick
+ fixture.componentInstance.scrollDown();
+ // Use tick(100) to tick forward to let stickyHeader's _applyStickyPositionStyles()
+ // function finished


Because after we calling fixture.componentInstance.scrollDown();, the scrollDown() function dispatches a fake scroll event, which causes the _applyStickyPositionStyles being called. And When it finds that the header needs to be stuck, it will reassign value to the header , so the header will be repainted. And we need to wait till animation has finished.
Changed the comments to //wait till animation has finished'

sllethe · 2017-07-31T23:13:42Z

+ it('should find sticky positioning is applied', () => {
+ let position = window.getComputedStyle(stickyHeader.element).position;
+ expect(position).not.toBe(null);
+ if (position != null) {


👍 Changed to expect(/sticky/i.test(position!)).toBe(true);

sllethe · 2017-07-31T23:15:37Z

+ -webkit-appearance: none;
+ -moz-appearance: none;
+ height: 300px;
+ overflow: auto;


👍 Changed the indent to +2

tinayuangao · 2017-08-01T18:13:09Z

+ });
+
+ it('should be able to find stickyParent', () => {
+ expect(stickyHeader.stickyParent).not.toBe(null);


Add a test case when there's no user-specified cdkStickyRegion?

👍 Added new test for ' no user-specified cdkStickyRegion'

sllethe · 2017-08-01T18:22:55Z

Comments Addressed 👍
Please take another look

jelbourn

LGTM, no further comment from me on this PR.

In another PR we should move this from lib/ to cdk/

jelbourn · 2017-08-02T00:55:49Z

+
+ it('should be able to find stickyParent', () => {
+ let p = stickyHeader.stickyParent;
+ expect(p).not.toBe(null);


You can use toBeNull()

👍 Changed 'toBe(null)' to 'toBeNull()';

change 'toBe(null)' to 'toBeNull()'

sllethe · 2017-08-02T17:32:19Z

Comments Addressed 👍
Please take another look

tinayuangao

LGTM

andrewseguin

LGTM

Cool ! tick(debounce time)

@Injectable

* # This is a combination of 9 commits. # This is the 1st commit message: # This is a combination of 11 commits. # This is the 1st commit message: add lib files for sticky-header add chose parent add support to 'optional 'cdkStickyRegion' input ' add app-demo for sticky-header fix bugs and deleted unused tag id in HTML files modify fix some code according to PR review comments change some format to pass TSlint check add '_' before private elements delete @Injectable for StickyHeaderDirective. Because we do not need @Injectable refine code encapsulate 'set style for element' change @input() Delete 'Observable.fromEvent(this.upperScrollableContainer, 'scroll')' add const STICK_START_CLASS and STICK_END_CLASS Add doc for [cdkStickyRegion] and 'unstuckElement()'. Delete 'detach()' function, add its content into 'ngOnDestroy()'. change 'MdStickyHeaderModule' to 'CdkStickyHeaderModule'; encapsulate reset css style operation for sticky header. delete unnecessary gloable variables delete global variable '_width' Add doc for 'sticker()' function. explained how it works. add more doc for 'sticker()', explaining 'isStuck' flag 2 space for indent # This is the commit message angular#2: fix # This is the commit message angular#3: delete sticky-header demo part from this branch # This is the commit message angular#4: revert firebase file # This is the commit message angular#5: change code according to comments in PR # This is the commit message angular#6: revert firbaserc # This is the commit message angular#7: revert demo-app.ts # This is the commit message angular#8: revert routes.ts # This is the commit message angular#9: revert demo-app-module.ts # This is the commit message angular#10: change # This is the commit message angular#11: fix the problem of : 'this.stickyParent' might be 'null' # This is the commit message angular#2: change doc # This is the commit message angular#3: Change the constructor of 'cdkStickyRegion' to 'constructor(public readonly _elementRef: ElementRef) { }' # This is the commit message angular#4: Added prefix 'mat-' for CSS class # This is the commit message angular#5: Delete 'public' before variables # This is the commit message angular#6: Object.assign isn't supported in IE11; use extendObject from src/lib/core/util. # This is the commit message angular#7: IE11 will have trouble with `translate3d(0, 0, 0);', change to `translate3d(0px, 0px, 0px);' # This is the commit message angular#8: Added docs for all variables # This is the commit message angular#9: extract 'generate CSS style' * # This is a combination of 5 commits. # This is the 1st commit message: add lib files for sticky-header add chose parent add support to 'optional 'cdkStickyRegion' input ' add app-demo for sticky-header fix bugs and deleted unused tag id in HTML files modify fix some code according to PR review comments change some format to pass TSlint check add '_' before private elements delete @Injectable for StickyHeaderDirective. Because we do not need @Injectable refine code encapsulate 'set style for element' change @input() Delete 'Observable.fromEvent(this.upperScrollableContainer, 'scroll')' add const STICK_START_CLASS and STICK_END_CLASS Add doc for [cdkStickyRegion] and 'unstuckElement()'. Delete 'detach()' function, add its content into 'ngOnDestroy()'. change 'MdStickyHeaderModule' to 'CdkStickyHeaderModule'; encapsulate reset css style operation for sticky header. delete unnecessary gloable variables delete global variable '_width' Add doc for 'sticker()' function. explained how it works. add more doc for 'sticker()', explaining 'isStuck' flag 2 space for indent fix delete sticky-header demo part from this branch revert firebase file change code according to comments in PR revert firbaserc revert demo-app.ts revert routes.ts revert demo-app-module.ts change fix the problem of : 'this.stickyParent' might be 'null' change doc Change the constructor of 'cdkStickyRegion' to 'constructor(public readonly _elementRef: ElementRef) { }' Added prefix 'mat-' for CSS class Delete 'public' before variables Object.assign isn't supported in IE11; use extendObject from src/lib/core/util. IE11 will have trouble with `translate3d(0, 0, 0);', change to `translate3d(0px, 0px, 0px);' Added docs for all variables extract 'generate CSS style' created a generateStyleCSS() function, let it be responsible for generating all those CSS styles. reformat add debounce to solve 'getBoundingClientRect() cause slow down' problem. add position:sticky and check whether browser support it. If not , use the naive implementation removed unused import Removed unused 'scrollableRegion' and 'parentRegion' removed commented lines default public Add comments about why setting style top and position for iPhone and not IE. And extract detectBrowser() as a new function format consider all circumstances of browser. use "===" instead of '==' make 'navigator.userAgent.toLocaleLowerCase()' a local variable optimize Added comments on const 'STICK_START_CLASS' and 'STICK_END_CLASS'. change their content to cdk-sticky-header-start and cdk-sticky-header-end Added comments for STICK_START_CLASS and STICK_END_CLASS. Changed the format of one-line JsDoc unsubscribe sbscriptions onDestory Use what modernizr does on compatibility instead of get the browser version directly. add 'padding' and 'stickyRegionHeight' variables to avoid calling 'getComputedStyle()' too many times (which is expensive). move docs above @directive removed the underscore in'_element: ElementRef', expand 'reg' to 'region' use 'if (this.isIE)' instead of 'if(this.isIE === true)' added more newlines between params in 'generateCssStyle()' function to make it easier to understand. Added reference link to Modernizer in docs of getSupportList() Deleted "_supportList" variable renamed 'isIE' to 'isStickyPositionSupported', and removed extra space before Observable Set debounce time as a const variable Added docs for 'const DEBOUNCE_TIME: number = 5;' Changed ' if(this.stickyParent == null)' to ' if(!this.stickyParent)' Removed the @PARAM and @returns and make sure the types are correct in the function signature in 'generateCssStyle(...)' function Added docs for `isStickyPositionSupported` variable changed '+=' to '=' of 'stickyText' in getSupportList() function nit added " " between 'if' and '(' nit Added comments deleted unused import change comments optimize comments deleted unnecessary global variables(padding and stickyRegionHeight) Added check whether we are on browser Array<string> to string[] test? try to reopen the old PR fix after rebase revert list.ts test test 222 revert demo revert list.ts second time Move code to 'src/cdk' revert 'move code to 'src/cdk'' , it should be done in a new PR revert avoid calling 'getComputedStyle()' too many times. rename as sticky-header.ts # This is the commit message angular#2: imported PlatformModule # This is the commit message angular#3: Add blank lines between these top-level symbol # This is the commit message angular#4: make '_isStickyPositionSupported' private # This is the commit message angular#5: Changed the originalCSS to private and use '{} as CSSStyleDeclaration' instead of ''any. * # This is a combination of 16 commits. # This is the 1st commit message: # This is a combination of 10 commits. # This is the 1st commit message: add lib files for sticky-header add chose parent add support to 'optional 'cdkStickyRegion' input ' add app-demo for sticky-header fix bugs and deleted unused tag id in HTML files modify fix some code according to PR review comments change some format to pass TSlint check add '_' before private elements delete @Injectable for StickyHeaderDirective. Because we do not need @Injectable refine code encapsulate 'set style for element' change @input() Delete 'Observable.fromEvent(this.upperScrollableContainer, 'scroll')' add const STICK_START_CLASS and STICK_END_CLASS Add doc for [cdkStickyRegion] and 'unstuckElement()'. Delete 'detach()' function, add its content into 'ngOnDestroy()'. change 'MdStickyHeaderModule' to 'CdkStickyHeaderModule'; encapsulate reset css style operation for sticky header. delete unnecessary gloable variables delete global variable '_width' Add doc for 'sticker()' function. explained how it works. add more doc for 'sticker()', explaining 'isStuck' flag 2 space for indent fix delete sticky-header demo part from this branch revert firebase file change code according to comments in PR revert firbaserc revert demo-app.ts revert routes.ts revert demo-app-module.ts change fix the problem of : 'this.stickyParent' might be 'null' change doc Change the constructor of 'cdkStickyRegion' to 'constructor(public readonly _elementRef: ElementRef) { }' Added prefix 'mat-' for CSS class Delete 'public' before variables Object.assign isn't supported in IE11; use extendObject from src/lib/core/util. IE11 will have trouble with `translate3d(0, 0, 0);', change to `translate3d(0px, 0px, 0px);' Added docs for all variables extract 'generate CSS style' created a generateStyleCSS() function, let it be responsible for generating all those CSS styles. reformat add debounce to solve 'getBoundingClientRect() cause slow down' problem. add position:sticky and check whether browser support it. If not , use the naive implementation removed unused import Removed unused 'scrollableRegion' and 'parentRegion' removed commented lines default public Add comments about why setting style top and position for iPhone and not IE. And extract detectBrowser() as a new function format consider all circumstances of browser. use "===" instead of '==' make 'navigator.userAgent.toLocaleLowerCase()' a local variable optimize Added comments on const 'STICK_START_CLASS' and 'STICK_END_CLASS'. change their content to cdk-sticky-header-start and cdk-sticky-header-end Added comments for STICK_START_CLASS and STICK_END_CLASS. Changed the format of one-line JsDoc unsubscribe sbscriptions onDestory Use what modernizr does on compatibility instead of get the browser version directly. add 'padding' and 'stickyRegionHeight' variables to avoid calling 'getComputedStyle()' too many times (which is expensive). move docs above @directive removed the underscore in'_element: ElementRef', expand 'reg' to 'region' use 'if (this.isIE)' instead of 'if(this.isIE === true)' added more newlines between params in 'generateCssStyle()' function to make it easier to understand. Added reference link to Modernizer in docs of getSupportList() Deleted "_supportList" variable renamed 'isIE' to 'isStickyPositionSupported', and removed extra space before Observable Set debounce time as a const variable Added docs for 'const DEBOUNCE_TIME: number = 5;' Changed ' if(this.stickyParent == null)' to ' if(!this.stickyParent)' Removed the @PARAM and @returns and make sure the types are correct in the function signature in 'generateCssStyle(...)' function Added docs for `isStickyPositionSupported` variable changed '+=' to '=' of 'stickyText' in getSupportList() function nit added " " between 'if' and '(' nit Added comments deleted unused import change comments optimize comments deleted unnecessary global variables(padding and stickyRegionHeight) Added check whether we are on browser Array<string> to string[] test? try to reopen the old PR fix after rebase revert list.ts test test 222 revert demo revert list.ts second time Move code to 'src/cdk' revert 'move code to 'src/cdk'' , it should be done in a new PR revert avoid calling 'getComputedStyle()' too many times. rename as sticky-header.ts imported PlatformModule Add blank lines between these top-level symbol make '_isStickyPositionSupported' private Changed the originalCSS to private and use '{} as CSSStyleDeclaration' instead of ''any. Rename '_containerStart' to '_stickyRegionTop' # This is the commit message angular#2: rename # This is the commit message angular#3: optimize discription for '_stickyRegionBottomThreshold' # This is the commit message angular#4: private _originalStyles = { position: '', top: '', right: '', left: '', bottom: '', width: '', zIndex: ''}; # This is the commit message angular#5: Deleted 'generateCssStyle()' and 'getCssNumber()' function # This is the commit message angular#6: Deleted 'getCssValue()' function # This is the commit message angular#7: fix CSSStyleDeclaration # This is the commit message angular#8: change sticky width to 'this.upperScrollableContainer.clientWidth' # This is the commit message angular#9: fix # This is the commit message angular#10: nit # This is the commit message angular#2: Added the 'isPositionStickySupported() ' function to src/cdk/platform/features.ts. Consume that function in this component and just always use both the webkit and unprefixed styles. # This is the commit message angular#3: nit # This is the commit message angular#4: nit # This is the commit message angular#5: update doc 'Debounce time in milliseconds for events that affect the sticky positioning (e.g. scroll, resize, touch move). Set as 5 milliseconds which is the highest delay that doesn't drastically affect the positioning adversely.' # This is the commit message angular#6: changed the doc to '/** z-index to be applied to the sticky header (default is 10). */' # This is the commit message angular#7: fix tslint error # This is the commit message angular#8: for comment 'Can you evaluate each method to make sure their accessor privacy is right? E.g. see which functions need to be public, private, static, etc' # This is the commit message angular#9: Deleted variable 'elemHeight' # This is the commit message angular#10: Chaned to 'if (!this.stickyParent)' # This is the commit message angular#11: Simplified Docs for 'sticker()'. # This is the commit message angular#12: set 'defineRestriction()' function to private # This is the commit message angular#13: use 'RxChain' # This is the commit message angular#14: deleted unused 'tableModule' in modules.ts # This is the commit message angular#15: rename to '_isPositionStickySupported' # This is the commit message angular#16: Use // for comments, /* */ for docs * add lib files for sticky-header add chose parent add support to 'optional 'cdkStickyRegion' input ' add app-demo for sticky-header fix bugs and deleted unused tag id in HTML files modify fix some code according to PR review comments change some format to pass TSlint check add '_' before private elements delete @Injectable for StickyHeaderDirective. Because we do not need @Injectable refine code encapsulate 'set style for element' change @input() Delete 'Observable.fromEvent(this.upperScrollableContainer, 'scroll')' add const STICK_START_CLASS and STICK_END_CLASS Add doc for [cdkStickyRegion] and 'unstuckElement()'. Delete 'detach()' function, add its content into 'ngOnDestroy()'. change 'MdStickyHeaderModule' to 'CdkStickyHeaderModule'; encapsulate reset css style operation for sticky header. delete unnecessary gloable variables delete global variable '_width' Add doc for 'sticker()' function. explained how it works. add more doc for 'sticker()', explaining 'isStuck' flag 2 space for indent fix delete sticky-header demo part from this branch revert firebase file change code according to comments in PR revert firbaserc revert demo-app.ts revert routes.ts revert demo-app-module.ts change fix the problem of : 'this.stickyParent' might be 'null' change doc Change the constructor of 'cdkStickyRegion' to 'constructor(public readonly _elementRef: ElementRef) { }' Added prefix 'mat-' for CSS class Delete 'public' before variables Object.assign isn't supported in IE11; use extendObject from src/lib/core/util. IE11 will have trouble with `translate3d(0, 0, 0);', change to `translate3d(0px, 0px, 0px);' Added docs for all variables extract 'generate CSS style' created a generateStyleCSS() function, let it be responsible for generating all those CSS styles. reformat add debounce to solve 'getBoundingClientRect() cause slow down' problem. add position:sticky and check whether browser support it. If not , use the naive implementation removed unused import Removed unused 'scrollableRegion' and 'parentRegion' removed commented lines default public Add comments about why setting style top and position for iPhone and not IE. And extract detectBrowser() as a new function format consider all circumstances of browser. use "===" instead of '==' make 'navigator.userAgent.toLocaleLowerCase()' a local variable optimize Added comments on const 'STICK_START_CLASS' and 'STICK_END_CLASS'. change their content to cdk-sticky-header-start and cdk-sticky-header-end Added comments for STICK_START_CLASS and STICK_END_CLASS. Changed the format of one-line JsDoc unsubscribe sbscriptions onDestory Use what modernizr does on compatibility instead of get the browser version directly. add 'padding' and 'stickyRegionHeight' variables to avoid calling 'getComputedStyle()' too many times (which is expensive). move docs above @directive removed the underscore in'_element: ElementRef', expand 'reg' to 'region' use 'if (this.isIE)' instead of 'if(this.isIE === true)' added more newlines between params in 'generateCssStyle()' function to make it easier to understand. Added reference link to Modernizer in docs of getSupportList() Deleted "_supportList" variable renamed 'isIE' to 'isStickyPositionSupported', and removed extra space before Observable Set debounce time as a const variable Added docs for 'const DEBOUNCE_TIME: number = 5;' Changed ' if(this.stickyParent == null)' to ' if(!this.stickyParent)' Removed the @PARAM and @returns and make sure the types are correct in the function signature in 'generateCssStyle(...)' function Added docs for `isStickyPositionSupported` variable changed '+=' to '=' of 'stickyText' in getSupportList() function nit added " " between 'if' and '(' nit Added comments deleted unused import change comments optimize comments deleted unnecessary global variables(padding and stickyRegionHeight) Added check whether we are on browser Array<string> to string[] test? try to reopen the old PR fix after rebase revert list.ts test test 222 revert demo revert list.ts second time Move code to 'src/cdk' revert 'move code to 'src/cdk'' , it should be done in a new PR revert avoid calling 'getComputedStyle()' too many times. rename as sticky-header.ts imported PlatformModule Add blank lines between these top-level symbol make '_isStickyPositionSupported' private Changed the originalCSS to private and use '{} as CSSStyleDeclaration' instead of ''any. Rename '_containerStart' to '_stickyRegionTop' rename optimize discription for '_stickyRegionBottomThreshold' private _originalStyles = { position: '', top: '', right: '', left: '', bottom: '', width: '', zIndex: ''}; Deleted 'generateCssStyle()' and 'getCssNumber()' function Deleted 'getCssValue()' function fix CSSStyleDeclaration change sticky width to 'this.upperScrollableContainer.clientWidth' fix nit Added the 'isPositionStickySupported() ' function to src/cdk/platform/features.ts. Consume that function in this component and just always use both the webkit and unprefixed styles. nit nit update doc 'Debounce time in milliseconds for events that affect the sticky positioning (e.g. scroll, resize, touch move). Set as 5 milliseconds which is the highest delay that doesn't drastically affect the positioning adversely.' changed the doc to '/** z-index to be applied to the sticky header (default is 10). */' fix tslint error for comment 'Can you evaluate each method to make sure their accessor privacy is right? E.g. see which functions need to be public, private, static, etc' Deleted variable 'elemHeight' Chaned to 'if (!this.stickyParent)' Simplified Docs for 'sticker()'. set 'defineRestriction()' function to private use 'RxChain' deleted unused 'tableModule' in modules.ts rename to '_isPositionStickySupported' Use // for comments, /* */ for docs @angular/cdk rename : values -> headerStyles Move closing brace to the next line optimized: [this._onScrollSubscription, this._onScrollSubscription, this._onResizeSubscription] .forEach(s => s && s.unsubscribe()); You should be able to do just '0' instead of '0px' Format like TODO(sllethe): ... private _attachEventListeners? Add a description like "Add listeners for events that affect sticky positioning." optimize doc Rename 'defineRestrictions' to '_measureStickyRegionBounds' rename: private _resetElementStyles let stuckRight: any = this.upperScrollableContainer.getBoundingClientRect().right; chaned 'any' to 'number' nit change doc '/** * Unsticks the header so that it goes back to scrolling normally. * * This should be called when the element reaches the bottom of its cdkStickyRegion so that it * smoothly scrolls out of view as the next sticky-header moves in. */' _unstuckElement -> _unstickElement rename 'sticker()' to '_applyStickyPositionStyles()' rename 'defineRestrictionsAndStick()' to '_updateStickyPositioning()' * added tests for sticky-header * deleted * removed 'deps' in providers * put provider in index * optimize provider * Removed unused 'fixture.detectChanges()' in test * fix * You can remove when sticky positioning is not supported since it's in describe * stickyHeaderDir.stickyParent * remove commented code * rename stickyHeaderDir to stickyHeader * Changed to use 'ngFor' to expand content instead of typing tons of '' * Add a comment that explains what these inline styles are for? Any reason you can't set them via the styles configuration for the component? * Added explainations on styles * Added /** @docs-private */ * Added comments to explaining why tick(100) is needed. Changed 'tick(0)' to 'tick()' * nit * explained why need 'tick(100)' * expect(/sticky/i.test(position!)).toBe(true); * change CSS indent to +2 * nit * Added test for sticky-header without StickyRegion * change 'toBe(null)' to 'toBeNull()' * Cool ! tick(debounce time)

@Injectable

* # This is a combination of 9 commits. # This is the 1st commit message: # This is a combination of 11 commits. # This is the 1st commit message: add lib files for sticky-header add chose parent add support to 'optional 'cdkStickyRegion' input ' add app-demo for sticky-header fix bugs and deleted unused tag id in HTML files modify fix some code according to PR review comments change some format to pass TSlint check add '_' before private elements delete @Injectable for StickyHeaderDirective. Because we do not need @Injectable refine code encapsulate 'set style for element' change @input() Delete 'Observable.fromEvent(this.upperScrollableContainer, 'scroll')' add const STICK_START_CLASS and STICK_END_CLASS Add doc for [cdkStickyRegion] and 'unstuckElement()'. Delete 'detach()' function, add its content into 'ngOnDestroy()'. change 'MdStickyHeaderModule' to 'CdkStickyHeaderModule'; encapsulate reset css style operation for sticky header. delete unnecessary gloable variables delete global variable '_width' Add doc for 'sticker()' function. explained how it works. add more doc for 'sticker()', explaining 'isStuck' flag 2 space for indent # This is the commit message angular#2: fix # This is the commit message angular#3: delete sticky-header demo part from this branch # This is the commit message angular#4: revert firebase file # This is the commit message angular#5: change code according to comments in PR # This is the commit message angular#6: revert firbaserc # This is the commit message angular#7: revert demo-app.ts # This is the commit message angular#8: revert routes.ts # This is the commit message angular#9: revert demo-app-module.ts # This is the commit message angular#10: change # This is the commit message angular#11: fix the problem of : 'this.stickyParent' might be 'null' # This is the commit message angular#2: change doc # This is the commit message angular#3: Change the constructor of 'cdkStickyRegion' to 'constructor(public readonly _elementRef: ElementRef) { }' # This is the commit message angular#4: Added prefix 'mat-' for CSS class # This is the commit message angular#5: Delete 'public' before variables # This is the commit message angular#6: Object.assign isn't supported in IE11; use extendObject from src/lib/core/util. # This is the commit message angular#7: IE11 will have trouble with `translate3d(0, 0, 0);', change to `translate3d(0px, 0px, 0px);' # This is the commit message angular#8: Added docs for all variables # This is the commit message angular#9: extract 'generate CSS style' * # This is a combination of 5 commits. # This is the 1st commit message: add lib files for sticky-header add chose parent add support to 'optional 'cdkStickyRegion' input ' add app-demo for sticky-header fix bugs and deleted unused tag id in HTML files modify fix some code according to PR review comments change some format to pass TSlint check add '_' before private elements delete @Injectable for StickyHeaderDirective. Because we do not need @Injectable refine code encapsulate 'set style for element' change @input() Delete 'Observable.fromEvent(this.upperScrollableContainer, 'scroll')' add const STICK_START_CLASS and STICK_END_CLASS Add doc for [cdkStickyRegion] and 'unstuckElement()'. Delete 'detach()' function, add its content into 'ngOnDestroy()'. change 'MdStickyHeaderModule' to 'CdkStickyHeaderModule'; encapsulate reset css style operation for sticky header. delete unnecessary gloable variables delete global variable '_width' Add doc for 'sticker()' function. explained how it works. add more doc for 'sticker()', explaining 'isStuck' flag 2 space for indent fix delete sticky-header demo part from this branch revert firebase file change code according to comments in PR revert firbaserc revert demo-app.ts revert routes.ts revert demo-app-module.ts change fix the problem of : 'this.stickyParent' might be 'null' change doc Change the constructor of 'cdkStickyRegion' to 'constructor(public readonly _elementRef: ElementRef) { }' Added prefix 'mat-' for CSS class Delete 'public' before variables Object.assign isn't supported in IE11; use extendObject from src/lib/core/util. IE11 will have trouble with `translate3d(0, 0, 0);', change to `translate3d(0px, 0px, 0px);' Added docs for all variables extract 'generate CSS style' created a generateStyleCSS() function, let it be responsible for generating all those CSS styles. reformat add debounce to solve 'getBoundingClientRect() cause slow down' problem. add position:sticky and check whether browser support it. If not , use the naive implementation removed unused import Removed unused 'scrollableRegion' and 'parentRegion' removed commented lines default public Add comments about why setting style top and position for iPhone and not IE. And extract detectBrowser() as a new function format consider all circumstances of browser. use "===" instead of '==' make 'navigator.userAgent.toLocaleLowerCase()' a local variable optimize Added comments on const 'STICK_START_CLASS' and 'STICK_END_CLASS'. change their content to cdk-sticky-header-start and cdk-sticky-header-end Added comments for STICK_START_CLASS and STICK_END_CLASS. Changed the format of one-line JsDoc unsubscribe sbscriptions onDestory Use what modernizr does on compatibility instead of get the browser version directly. add 'padding' and 'stickyRegionHeight' variables to avoid calling 'getComputedStyle()' too many times (which is expensive). move docs above @directive removed the underscore in'_element: ElementRef', expand 'reg' to 'region' use 'if (this.isIE)' instead of 'if(this.isIE === true)' added more newlines between params in 'generateCssStyle()' function to make it easier to understand. Added reference link to Modernizer in docs of getSupportList() Deleted "_supportList" variable renamed 'isIE' to 'isStickyPositionSupported', and removed extra space before Observable Set debounce time as a const variable Added docs for 'const DEBOUNCE_TIME: number = 5;' Changed ' if(this.stickyParent == null)' to ' if(!this.stickyParent)' Removed the @PARAM and @returns and make sure the types are correct in the function signature in 'generateCssStyle(...)' function Added docs for `isStickyPositionSupported` variable changed '+=' to '=' of 'stickyText' in getSupportList() function nit added " " between 'if' and '(' nit Added comments deleted unused import change comments optimize comments deleted unnecessary global variables(padding and stickyRegionHeight) Added check whether we are on browser Array<string> to string[] test? try to reopen the old PR fix after rebase revert list.ts test test 222 revert demo revert list.ts second time Move code to 'src/cdk' revert 'move code to 'src/cdk'' , it should be done in a new PR revert avoid calling 'getComputedStyle()' too many times. rename as sticky-header.ts # This is the commit message angular#2: imported PlatformModule # This is the commit message angular#3: Add blank lines between these top-level symbol # This is the commit message angular#4: make '_isStickyPositionSupported' private # This is the commit message angular#5: Changed the originalCSS to private and use '{} as CSSStyleDeclaration' instead of ''any. * # This is a combination of 16 commits. # This is the 1st commit message: # This is a combination of 10 commits. # This is the 1st commit message: add lib files for sticky-header add chose parent add support to 'optional 'cdkStickyRegion' input ' add app-demo for sticky-header fix bugs and deleted unused tag id in HTML files modify fix some code according to PR review comments change some format to pass TSlint check add '_' before private elements delete @Injectable for StickyHeaderDirective. Because we do not need @Injectable refine code encapsulate 'set style for element' change @input() Delete 'Observable.fromEvent(this.upperScrollableContainer, 'scroll')' add const STICK_START_CLASS and STICK_END_CLASS Add doc for [cdkStickyRegion] and 'unstuckElement()'. Delete 'detach()' function, add its content into 'ngOnDestroy()'. change 'MdStickyHeaderModule' to 'CdkStickyHeaderModule'; encapsulate reset css style operation for sticky header. delete unnecessary gloable variables delete global variable '_width' Add doc for 'sticker()' function. explained how it works. add more doc for 'sticker()', explaining 'isStuck' flag 2 space for indent fix delete sticky-header demo part from this branch revert firebase file change code according to comments in PR revert firbaserc revert demo-app.ts revert routes.ts revert demo-app-module.ts change fix the problem of : 'this.stickyParent' might be 'null' change doc Change the constructor of 'cdkStickyRegion' to 'constructor(public readonly _elementRef: ElementRef) { }' Added prefix 'mat-' for CSS class Delete 'public' before variables Object.assign isn't supported in IE11; use extendObject from src/lib/core/util. IE11 will have trouble with `translate3d(0, 0, 0);', change to `translate3d(0px, 0px, 0px);' Added docs for all variables extract 'generate CSS style' created a generateStyleCSS() function, let it be responsible for generating all those CSS styles. reformat add debounce to solve 'getBoundingClientRect() cause slow down' problem. add position:sticky and check whether browser support it. If not , use the naive implementation removed unused import Removed unused 'scrollableRegion' and 'parentRegion' removed commented lines default public Add comments about why setting style top and position for iPhone and not IE. And extract detectBrowser() as a new function format consider all circumstances of browser. use "===" instead of '==' make 'navigator.userAgent.toLocaleLowerCase()' a local variable optimize Added comments on const 'STICK_START_CLASS' and 'STICK_END_CLASS'. change their content to cdk-sticky-header-start and cdk-sticky-header-end Added comments for STICK_START_CLASS and STICK_END_CLASS. Changed the format of one-line JsDoc unsubscribe sbscriptions onDestory Use what modernizr does on compatibility instead of get the browser version directly. add 'padding' and 'stickyRegionHeight' variables to avoid calling 'getComputedStyle()' too many times (which is expensive). move docs above @directive removed the underscore in'_element: ElementRef', expand 'reg' to 'region' use 'if (this.isIE)' instead of 'if(this.isIE === true)' added more newlines between params in 'generateCssStyle()' function to make it easier to understand. Added reference link to Modernizer in docs of getSupportList() Deleted "_supportList" variable renamed 'isIE' to 'isStickyPositionSupported', and removed extra space before Observable Set debounce time as a const variable Added docs for 'const DEBOUNCE_TIME: number = 5;' Changed ' if(this.stickyParent == null)' to ' if(!this.stickyParent)' Removed the @PARAM and @returns and make sure the types are correct in the function signature in 'generateCssStyle(...)' function Added docs for `isStickyPositionSupported` variable changed '+=' to '=' of 'stickyText' in getSupportList() function nit added " " between 'if' and '(' nit Added comments deleted unused import change comments optimize comments deleted unnecessary global variables(padding and stickyRegionHeight) Added check whether we are on browser Array<string> to string[] test? try to reopen the old PR fix after rebase revert list.ts test test 222 revert demo revert list.ts second time Move code to 'src/cdk' revert 'move code to 'src/cdk'' , it should be done in a new PR revert avoid calling 'getComputedStyle()' too many times. rename as sticky-header.ts imported PlatformModule Add blank lines between these top-level symbol make '_isStickyPositionSupported' private Changed the originalCSS to private and use '{} as CSSStyleDeclaration' instead of ''any. Rename '_containerStart' to '_stickyRegionTop' # This is the commit message angular#2: rename # This is the commit message angular#3: optimize discription for '_stickyRegionBottomThreshold' # This is the commit message angular#4: private _originalStyles = { position: '', top: '', right: '', left: '', bottom: '', width: '', zIndex: ''}; # This is the commit message angular#5: Deleted 'generateCssStyle()' and 'getCssNumber()' function # This is the commit message angular#6: Deleted 'getCssValue()' function # This is the commit message angular#7: fix CSSStyleDeclaration # This is the commit message angular#8: change sticky width to 'this.upperScrollableContainer.clientWidth' # This is the commit message angular#9: fix # This is the commit message angular#10: nit # This is the commit message angular#2: Added the 'isPositionStickySupported() ' function to src/cdk/platform/features.ts. Consume that function in this component and just always use both the webkit and unprefixed styles. # This is the commit message angular#3: nit # This is the commit message angular#4: nit # This is the commit message angular#5: update doc 'Debounce time in milliseconds for events that affect the sticky positioning (e.g. scroll, resize, touch move). Set as 5 milliseconds which is the highest delay that doesn't drastically affect the positioning adversely.' # This is the commit message angular#6: changed the doc to '/** z-index to be applied to the sticky header (default is 10). */' # This is the commit message angular#7: fix tslint error # This is the commit message angular#8: for comment 'Can you evaluate each method to make sure their accessor privacy is right? E.g. see which functions need to be public, private, static, etc' # This is the commit message angular#9: Deleted variable 'elemHeight' # This is the commit message angular#10: Chaned to 'if (!this.stickyParent)' # This is the commit message angular#11: Simplified Docs for 'sticker()'. # This is the commit message angular#12: set 'defineRestriction()' function to private # This is the commit message angular#13: use 'RxChain' # This is the commit message angular#14: deleted unused 'tableModule' in modules.ts # This is the commit message angular#15: rename to '_isPositionStickySupported' # This is the commit message angular#16: Use // for comments, /* */ for docs * add lib files for sticky-header add chose parent add support to 'optional 'cdkStickyRegion' input ' add app-demo for sticky-header fix bugs and deleted unused tag id in HTML files modify fix some code according to PR review comments change some format to pass TSlint check add '_' before private elements delete @Injectable for StickyHeaderDirective. Because we do not need @Injectable refine code encapsulate 'set style for element' change @input() Delete 'Observable.fromEvent(this.upperScrollableContainer, 'scroll')' add const STICK_START_CLASS and STICK_END_CLASS Add doc for [cdkStickyRegion] and 'unstuckElement()'. Delete 'detach()' function, add its content into 'ngOnDestroy()'. change 'MdStickyHeaderModule' to 'CdkStickyHeaderModule'; encapsulate reset css style operation for sticky header. delete unnecessary gloable variables delete global variable '_width' Add doc for 'sticker()' function. explained how it works. add more doc for 'sticker()', explaining 'isStuck' flag 2 space for indent fix delete sticky-header demo part from this branch revert firebase file change code according to comments in PR revert firbaserc revert demo-app.ts revert routes.ts revert demo-app-module.ts change fix the problem of : 'this.stickyParent' might be 'null' change doc Change the constructor of 'cdkStickyRegion' to 'constructor(public readonly _elementRef: ElementRef) { }' Added prefix 'mat-' for CSS class Delete 'public' before variables Object.assign isn't supported in IE11; use extendObject from src/lib/core/util. IE11 will have trouble with `translate3d(0, 0, 0);', change to `translate3d(0px, 0px, 0px);' Added docs for all variables extract 'generate CSS style' created a generateStyleCSS() function, let it be responsible for generating all those CSS styles. reformat add debounce to solve 'getBoundingClientRect() cause slow down' problem. add position:sticky and check whether browser support it. If not , use the naive implementation removed unused import Removed unused 'scrollableRegion' and 'parentRegion' removed commented lines default public Add comments about why setting style top and position for iPhone and not IE. And extract detectBrowser() as a new function format consider all circumstances of browser. use "===" instead of '==' make 'navigator.userAgent.toLocaleLowerCase()' a local variable optimize Added comments on const 'STICK_START_CLASS' and 'STICK_END_CLASS'. change their content to cdk-sticky-header-start and cdk-sticky-header-end Added comments for STICK_START_CLASS and STICK_END_CLASS. Changed the format of one-line JsDoc unsubscribe sbscriptions onDestory Use what modernizr does on compatibility instead of get the browser version directly. add 'padding' and 'stickyRegionHeight' variables to avoid calling 'getComputedStyle()' too many times (which is expensive). move docs above @directive removed the underscore in'_element: ElementRef', expand 'reg' to 'region' use 'if (this.isIE)' instead of 'if(this.isIE === true)' added more newlines between params in 'generateCssStyle()' function to make it easier to understand. Added reference link to Modernizer in docs of getSupportList() Deleted "_supportList" variable renamed 'isIE' to 'isStickyPositionSupported', and removed extra space before Observable Set debounce time as a const variable Added docs for 'const DEBOUNCE_TIME: number = 5;' Changed ' if(this.stickyParent == null)' to ' if(!this.stickyParent)' Removed the @PARAM and @returns and make sure the types are correct in the function signature in 'generateCssStyle(...)' function Added docs for `isStickyPositionSupported` variable changed '+=' to '=' of 'stickyText' in getSupportList() function nit added " " between 'if' and '(' nit Added comments deleted unused import change comments optimize comments deleted unnecessary global variables(padding and stickyRegionHeight) Added check whether we are on browser Array<string> to string[] test? try to reopen the old PR fix after rebase revert list.ts test test 222 revert demo revert list.ts second time Move code to 'src/cdk' revert 'move code to 'src/cdk'' , it should be done in a new PR revert avoid calling 'getComputedStyle()' too many times. rename as sticky-header.ts imported PlatformModule Add blank lines between these top-level symbol make '_isStickyPositionSupported' private Changed the originalCSS to private and use '{} as CSSStyleDeclaration' instead of ''any. Rename '_containerStart' to '_stickyRegionTop' rename optimize discription for '_stickyRegionBottomThreshold' private _originalStyles = { position: '', top: '', right: '', left: '', bottom: '', width: '', zIndex: ''}; Deleted 'generateCssStyle()' and 'getCssNumber()' function Deleted 'getCssValue()' function fix CSSStyleDeclaration change sticky width to 'this.upperScrollableContainer.clientWidth' fix nit Added the 'isPositionStickySupported() ' function to src/cdk/platform/features.ts. Consume that function in this component and just always use both the webkit and unprefixed styles. nit nit update doc 'Debounce time in milliseconds for events that affect the sticky positioning (e.g. scroll, resize, touch move). Set as 5 milliseconds which is the highest delay that doesn't drastically affect the positioning adversely.' changed the doc to '/** z-index to be applied to the sticky header (default is 10). */' fix tslint error for comment 'Can you evaluate each method to make sure their accessor privacy is right? E.g. see which functions need to be public, private, static, etc' Deleted variable 'elemHeight' Chaned to 'if (!this.stickyParent)' Simplified Docs for 'sticker()'. set 'defineRestriction()' function to private use 'RxChain' deleted unused 'tableModule' in modules.ts rename to '_isPositionStickySupported' Use // for comments, /* */ for docs @angular/cdk rename : values -> headerStyles Move closing brace to the next line optimized: [this._onScrollSubscription, this._onScrollSubscription, this._onResizeSubscription] .forEach(s => s && s.unsubscribe()); You should be able to do just '0' instead of '0px' Format like TODO(sllethe): ... private _attachEventListeners? Add a description like "Add listeners for events that affect sticky positioning." optimize doc Rename 'defineRestrictions' to '_measureStickyRegionBounds' rename: private _resetElementStyles let stuckRight: any = this.upperScrollableContainer.getBoundingClientRect().right; chaned 'any' to 'number' nit change doc '/** * Unsticks the header so that it goes back to scrolling normally. * * This should be called when the element reaches the bottom of its cdkStickyRegion so that it * smoothly scrolls out of view as the next sticky-header moves in. */' _unstuckElement -> _unstickElement rename 'sticker()' to '_applyStickyPositionStyles()' rename 'defineRestrictionsAndStick()' to '_updateStickyPositioning()' * added tests for sticky-header * deleted * removed 'deps' in providers * put provider in index * optimize provider * Removed unused 'fixture.detectChanges()' in test * fix * You can remove when sticky positioning is not supported since it's in describe * stickyHeaderDir.stickyParent * remove commented code * rename stickyHeaderDir to stickyHeader * Changed to use 'ngFor' to expand content instead of typing tons of '' * Add a comment that explains what these inline styles are for? Any reason you can't set them via the styles configuration for the component? * Added explainations on styles * Added /** @docs-private */ * Added comments to explaining why tick(100) is needed. Changed 'tick(0)' to 'tick()' * nit * explained why need 'tick(100)' * expect(/sticky/i.test(position!)).toBe(true); * change CSS indent to +2 * nit * Added test for sticky-header without StickyRegion * change 'toBe(null)' to 'toBeNull()' * Cool ! tick(debounce time)

angular-automatic-lock-bot · 2019-09-06T20:02:21Z

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}

sllethe added 6 commits July 26, 2017 15:32

added tests for sticky-header
b0172d1

d354ef0
deleted

googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Jul 26, 2017

sllethe requested a review from tinayuangao July 26, 2017 22:35

sllethe assigned tinayuangao Jul 26, 2017

tinayuangao reviewed Jul 26, 2017
View reviewed changes

sllethe added 4 commits July 26, 2017 17:04

6e58b23
removed 'deps' in providers

32b1b93
put provider in index

optimize provider
a555a23

469036d
Removed unused 'fixture.detectChanges()' in test

sllethe commented Jul 27, 2017
View reviewed changes

fix
4100311

sllethe requested a review from andrewseguin July 27, 2017 17:18

sllethe assigned andrewseguin Jul 27, 2017

sllethe requested a review from jelbourn July 27, 2017 17:19

sllethe assigned jelbourn Jul 27, 2017

sllethe added the pr: needs review label Jul 27, 2017

tinayuangao reviewed Jul 27, 2017
View reviewed changes

sllethe added 3 commits July 27, 2017 11:55

44201f0
You can remove when sticky positioning is not supported since it's in describe

4aa498a
stickyHeaderDir.stickyParent

4adcd52
remove commented code

jelbourn reviewed Jul 29, 2017
View reviewed changes

sllethe added 3 commits July 31, 2017 10:59

rename stickyHeaderDir to stickyHeader
783274c

9c28f34
Changed to use 'ngFor' to expand content instead of typing tons of ''

b037d83
Add a comment that explains what these inline styles are for? Any reason you can't set them via the styles configuration for the component?

sllethe added 2 commits July 31, 2017 11:45

13d7aca
Added explainations on styles

824b305
Added /** @docs-private */

sllethe commented Jul 31, 2017
View reviewed changes

72706d8
Added comments to explaining why tick(100) is needed. Changed 'tick(0)' to 'tick()'

sllethe commented Jul 31, 2017
View reviewed changes

jelbourn reviewed Jul 31, 2017
View reviewed changes

sllethe added 4 commits July 31, 2017 15:52

886b622
nit

089a0ca
explained why need 'tick(100)'

3391650
expect(/sticky/i.test(position!)).toBe(true);

change CSS indent to +2
54dccf6

sllethe commented Jul 31, 2017
View reviewed changes

nit
cc53d28

tinayuangao reviewed Aug 1, 2017
View reviewed changes

Added test for sticky-header without StickyRegion
0abc5ee

jelbourn approved these changes Aug 2, 2017
View reviewed changes

b943a62
change 'toBe(null)' to 'toBeNull()'

tinayuangao approved these changes Aug 2, 2017
View reviewed changes

andrewseguin approved these changes Aug 4, 2017
View reviewed changes

andrewseguin added pr: lgtm action: merge The PR is ready for merge by the caretaker and removed pr: needs review labels Aug 4, 2017

94e6474
Cool ! tick(debounce time)

sllethe merged commit c8185fb into angular:sticky-headerAug 4, 2017

angular-automatic-lock-botBot unassigned tinayuangao Sep 6, 2019

angular-automatic-lock-botBot locked and limited conversation to collaborators Sep 6, 2019


		export const STICKY_HEADER_SUPPORT_STRATEGY = new InjectionToken('sticky-header-support-strategy');

		/** Create a factory for sticky-positioning check to make code more testable */

Uh oh!

Conversation

sllethe commented Jul 26, 2017• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sllethe left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sllethe commented Jul 27, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sllethe left a comment

Choose a reason for hiding this comment

sllethe commented Jul 26, 2017•
edited
Loading