Skip to content
This repository has been archived by the owner on Dec 8, 2022. It is now read-only.

Added row iterator properties #10

Merged
merged 12 commits into from
Dec 20, 2018
32 changes: 32 additions & 0 deletions e2e/flyout.e2e-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,4 +47,36 @@ describe('Flyout', () => {
expect('body').toMatchBaselineScreenshot(done);
element(by.css('.sky-flyout .sky-flyout-btn-close')).click();
});

it('should match previous screenshot when row iterators are enabled', (done) => {
SkyHostBrowser.get('visual/flyout');
SkyHostBrowser.setWindowBreakpoint('lg');
element(by.css('#open-flyout-with-iterators')).click();
expect('body').toMatchBaselineScreenshot(done);
element(by.css('.sky-flyout .sky-flyout-btn-close')).click();
});

it('should match previous screenshot when row iterators are enabled (screen: xs)', (done) => {
SkyHostBrowser.get('visual/flyout');
SkyHostBrowser.setWindowBreakpoint('xs');
element(by.css('#open-flyout-with-iterators')).click();
expect('body').toMatchBaselineScreenshot(done);
element(by.css('.sky-flyout .sky-flyout-btn-close')).click();
});

it('should match previous screenshot when row iterators are disabled', (done) => {
SkyHostBrowser.get('visual/flyout');
SkyHostBrowser.setWindowBreakpoint('lg');
element(by.css('#open-flyout-with-iterators-disabled')).click();
expect('body').toMatchBaselineScreenshot(done);
element(by.css('.sky-flyout .sky-flyout-btn-close')).click();
});

it('should match previous screenshot when row iterator are disabled (screen: xs)', (done) => {
SkyHostBrowser.get('visual/flyout');
SkyHostBrowser.setWindowBreakpoint('xs');
element(by.css('#open-flyout-with-iterators-disabled')).click();
expect('body').toMatchBaselineScreenshot(done);
element(by.css('.sky-flyout .sky-flyout-btn-close')).click();
});
});
4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,8 @@
},
"dependencies": {},
"devDependencies": {
"@blackbaud/skyux": "2.28.1",
"@blackbaud/skyux-builder": "1.28.0",
"@blackbaud/skyux": "2.34.0",
"@blackbaud/skyux-builder": "1.30.0",
"@skyux-sdk/builder-plugin-skyux": "1.0.0-rc.5"
}
}
Binary file removed screenshots-baseline/body0-chrome-1044x788-dpr-1.png
Binary file not shown.
Binary file removed screenshots-baseline/body2-chrome-1044x788-dpr-1.png
Binary file not shown.
15 changes: 13 additions & 2 deletions src/app/public/modules/flyout/flyout-instance.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import {
EventEmitter
EventEmitter,
OnDestroy
} from '@angular/core';

import {
Expand All @@ -11,11 +12,16 @@ import {
SkyFlyoutMessageType
} from './types';

export class SkyFlyoutInstance<T> {
export class SkyFlyoutInstance<T> implements OnDestroy {
public closed = new EventEmitter<void>();
public componentInstance: T;
public isOpen = true;

public iterator = {
Blackbaud-SteveBrush marked this conversation as resolved.
Show resolved Hide resolved
previousButtonClick: new EventEmitter<void>(),
nextButtonClick: new EventEmitter<void>()
};

// Used to communicate with the host component.
public get hostController(): Subject<SkyFlyoutMessage> {
return this._hostController;
Expand All @@ -29,6 +35,11 @@ export class SkyFlyoutInstance<T> {
});
}

public ngOnDestroy() {
this.iterator.previousButtonClick.complete();
this.iterator.nextButtonClick.complete();
}

public close() {
this.hostController.next({
type: SkyFlyoutMessageType.Close
Expand Down
31 changes: 29 additions & 2 deletions src/app/public/modules/flyout/flyout.component.html
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,35 @@
(mousedown)="onMouseDown($event)" />

<div class="sky-flyout-header sky-padding-squish-large" #flyoutHeader>
<div class="sky-flyout-header-content"></div>
<div class="sky-flyout-header-buttons">
<div class="sky-flyout-header-content">

<div *ngIf="config?.iterator"
id="row-iterators"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Curious why you need an id on this element? If so, it will need an auto-increment applied to keep it unique.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was meant as a unique identifier to easily locate the buttons in the unit test. Isn't there only ever one flyout open at a time?

>
<button
class="sky-btn sky-btn-default"
[attr.aria-label]="'skyux_flyout_iterator_previous_button' | skyLibResources"
[disabled]="config.iterator.previousButtonDisabled"
(click)="onIteratorPreviousClick()"
>
<sky-icon
icon="chevron-up"
></sky-icon>
</button>
<button
class="sky-btn sky-btn-default"
[attr.aria-label]="'skyux_flyout_iterator_next_button' | skyLibResources"
[disabled]="config.iterator.nextButtonDisabled"
(click)="onIteratorNextClick()"
>
<sky-icon
icon="chevron-down"
></sky-icon>
</button>
</div>

</div>
<div class="sky-flyout-header-buttons">

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There might be a few extra tabs in the template, here.

<ng-container *ngTemplateOutlet="permalinkTemplate">
</ng-container>
<ng-container *ngTemplateOutlet="primaryActionTemplate">
Expand Down
120 changes: 120 additions & 0 deletions src/app/public/modules/flyout/flyout.component.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -652,4 +652,124 @@ describe('Flyout component', () => {
})
);
});

describe('row iterator', () => {
function getIteratorButtons() {
return document.querySelectorAll('#row-iterators button') as NodeListOf<HTMLButtonElement>;
}

it('should not show iterator buttons if config.iterator is undefined', fakeAsync(() => {
openFlyout();
const iteratorButtons = getIteratorButtons();
expect(iteratorButtons.length).toEqual(0);
}));

it('should show iterator buttons if config.iterator is defined', fakeAsync(() => {
openFlyout({
iterator: {
previousButtonDisabled: false,
nextButtonDisabled: false
}
});
const iteratorButtons = getIteratorButtons();
expect(iteratorButtons.length).toEqual(2);
expect(iteratorButtons[0].disabled).toBeFalsy();
expect(iteratorButtons[1].disabled).toBeFalsy();
}));

it('should disable iterator buttons if config.iterator contains truthy isDisabled properties', fakeAsync(() => {
openFlyout({
iterator: {
previousButtonDisabled: true,
nextButtonDisabled: true
}
});
const iteratorButtons = getIteratorButtons();
expect(iteratorButtons.length).toEqual(2);
expect(iteratorButtons[0].disabled).toBeTruthy();
expect(iteratorButtons[1].disabled).toBeTruthy();
}));

it('should emit if previous button is clicked', fakeAsync(() => {
let previousCalled = false;
let nextCalled = false;

const flyoutInstance = openFlyout({
iterator: {
previousButtonDisabled: false,
nextButtonDisabled: false
}
});

flyoutInstance.iterator.previousButtonClick.subscribe(() => {
previousCalled = true;
});

flyoutInstance.iterator.nextButtonClick.subscribe(() => {
nextCalled = true;
});

const iteratorButtons = getIteratorButtons();
iteratorButtons[0].click();
fixture.detectChanges();
tick();
expect(previousCalled).toEqual(true);
expect(nextCalled).toEqual(false);
}));

it('should emit if next button is clicked', fakeAsync(() => {
let previousCalled = false;
let nextCalled = false;

const flyoutInstance = openFlyout({
iterator: {
previousButtonDisabled: false,
nextButtonDisabled: false
}
});

flyoutInstance.iterator.previousButtonClick.subscribe(() => {
previousCalled = true;
});

flyoutInstance.iterator.nextButtonClick.subscribe(() => {
nextCalled = true;
});

const iteratorButtons = getIteratorButtons();
iteratorButtons[1].click();
fixture.detectChanges();
tick();
expect(previousCalled).toEqual(false);
expect(nextCalled).toEqual(true);
}));

it('should not emit if iterator buttons are clicked when config properties are disabled', fakeAsync(() => {
let previousCalled = false;
let nextCalled = false;

const flyoutInstance = openFlyout({
iterator: {
previousButtonDisabled: true,
nextButtonDisabled: true
}
});

flyoutInstance.iterator.previousButtonClick.subscribe(() => {
previousCalled = true;
});

flyoutInstance.iterator.nextButtonClick.subscribe(() => {
nextCalled = true;
});

const iteratorButtons = getIteratorButtons();
iteratorButtons[0].click();
iteratorButtons[1].click();
fixture.detectChanges();
tick();
expect(previousCalled).toEqual(false);
expect(nextCalled).toEqual(false);
}));
});
});
12 changes: 12 additions & 0 deletions src/app/public/modules/flyout/flyout.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -251,6 +251,18 @@ export class SkyFlyoutComponent implements OnDestroy, OnInit {
this.adapter.toggleIframePointerEvents(true);
}

public onIteratorPreviousClick() {
if (this.config.iterator && !this.config.iterator.previousButtonDisabled) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since these methods are isolated to the buttons' click events, do we need the conditions? For these events to be executed, the iterator configuration must already exist, and the event won't be triggered if the button is natively disabled. You might be safe to simply call the emit methods without any condition.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah true, but these are public methods. I think I was just adding extra safeguards here in case they were used for something else in the future. Could I persuade you on this one in the interest of being extra cautious?

this.flyoutInstance.iterator.previousButtonClick.emit();
}
}

public onIteratorNextClick() {
if (this.config.iterator && !this.config.iterator.nextButtonDisabled) {
this.flyoutInstance.iterator.nextButtonClick.emit();
}
}

private createFlyoutInstance<T>(component: T): SkyFlyoutInstance<T> {
const instance = new SkyFlyoutInstance<T>();

Expand Down
5 changes: 5 additions & 0 deletions src/app/public/modules/flyout/types/flyout-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@ import {
SkyFlyoutPermalink
} from './flyout-permalink';

import {
SkyFlyoutIterator
} from './flyout-iterator';

export interface SkyFlyoutConfig {
ariaDescribedBy?: string;
ariaLabelledBy?: string;
Expand All @@ -16,4 +20,5 @@ export interface SkyFlyoutConfig {
permalink?: SkyFlyoutPermalink;
primaryAction?: SkyFlyoutAction;
providers?: any[];
iterator?: SkyFlyoutIterator;
}
4 changes: 4 additions & 0 deletions src/app/public/modules/flyout/types/flyout-iterator.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
export interface SkyFlyoutIterator {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should rename this to SkyFlyoutIteratorConfig so that we can use the SkyFlyoutIterator type for the public SkyFlyoutInstance.iterator property?

previousButtonDisabled: boolean;
nextButtonDisabled: boolean;
}
1 change: 1 addition & 0 deletions src/app/public/modules/flyout/types/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,4 @@ export * from './flyout-message';
export * from './flyout-message-type';
export * from './flyout-action';
export * from './flyout-permalink';
export * from './flyout-iterator';
23 changes: 22 additions & 1 deletion src/app/visual/flyout/flyout-visual.component.html
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,32 @@
class="sky-flyout-demo-item"
>
<button
type="button"
class="sky-btn sky-btn-primary"
type="button"
(click)="openFlyout(user)"
>
{{ user.name }}
</button>
</div>

<div
class="sky-flyout-demo-item"
>
<button
class="sky-btn sky-btn-primary"
id="open-flyout-with-iterators"
type="button"
(click)="openFlyoutWithIterators(users[0], false, false)"
>
Open flyout with row iterators enabled
</button>
<button
class="sky-btn sky-btn-primary"
id="open-flyout-with-iterators-disabled"
type="button"
(click)="openFlyoutWithIterators(users[0], true, true)"
>
Open flyout with row iterators disabled
</button>
</div>
</div>
Loading