Fixed dropdown positioning lag; bad unit test #1655
Conversation
Blackbaud-SteveBrush
commented
Apr 25, 2018
Blackbaud-SteveBrush
commented
- The dropdown repositions itself on scroll and resize events. On the master branch, there is a noticeable "lag" when it's rewriting the top and left pixel values. This branch fixes that.
- I also noticed a "bad" unit test for the Timepicker.
| @@ -23,19 +22,20 @@ describe('Dropdown component', () => { | |||
| let fixture: ComponentFixture<DropdownTestComponent>; | |||
| let component: DropdownTestComponent; | |||
|
|
|||
| beforeEach(async(() => { | |||
| beforeEach(() => { | |||
There was a problem hiding this comment.
This change is inconsequential; compileComponents() is not needed when running in AoT mode.
| @@ -342,8 +342,7 @@ describe('SkyPopoverComponent', () => { | |||
| expect(component.placement).toEqual('right'); | |||
|
|
|||
| component.reposition(); | |||
| expect(component.placement).toEqual('above'); | |||
| tick(); | |||
| expect(spy.calls.argsFor(1)[1]).toEqual('above'); | |||
There was a problem hiding this comment.
This is a better way to write this test: checking which value is provided to getPopoverPosition().
| @@ -160,13 +160,11 @@ export class SkyPopoverComponent implements OnInit, OnDestroy { | |||
| this.placement = this.preferredPlacement; | |||
| this.changeDetector.markForCheck(); | |||
|
|
|||
| this.windowRef.getWindow().setTimeout(() => { | |||
There was a problem hiding this comment.
This is the line that was causing the lag; it's running a setTimeout during every reposition call (not good).
| const dropdown = fixture.nativeElement.querySelector('.sky-popover-container') as HTMLElement; | ||
| expect(dropdown.classList.contains('sky-popover-hidden')).toEqual(false); | ||
| const hiddenPopover = fixture.nativeElement.querySelector('.sky-popover-hidden') as HTMLElement; | ||
| expect(hiddenPopover).not.toBeNull(); |
There was a problem hiding this comment.
I found this bad test during my work on this bugfix. We were supposed to be checking if the popover had closed!
Codecov Report
@@ Coverage Diff @@
## master #1655 +/- ##
=======================================
Coverage 99.98% 99.98%
=======================================
Files 395 395
Lines 8108 8108
Branches 1194 1194
=======================================
Hits 8107 8107
Misses 1 1
Continue to review full report at Codecov.
|
