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

Add help button to modal header #1144

Merged
Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
c91b21a
added the header icon to the modal
Blackbaud-BrandonJones Sep 28, 2017
d0b09e4
Merge branch 'master' of github.com:blackbaud/skyux2 into feature-hel…
Blackbaud-BrandonJones Sep 28, 2017
ea716d5
tests
Blackbaud-BrandonJones Sep 29, 2017
e108ae9
added test, clearing old instance after open
Blackbaud-BrandonJones Sep 29, 2017
e364107
created demo and documents
Blackbaud-BrandonJones Sep 29, 2017
0e0dca0
visual tests for help header
Blackbaud-BrandonJones Sep 29, 2017
e1d4ce5
re-worded to allow for line length
Blackbaud-BrandonJones Sep 29, 2017
08da56c
Merge branch 'master' into feature-help-modal-header
Blackbaud-BrandonJones Sep 29, 2017
b36a567
Merge branch 'master' of github.com:blackbaud/skyux2 into feature-hel…
Blackbaud-BrandonJones Sep 29, 2017
6c9edec
better visual representation of the modal with a helpKey being clicke…
Blackbaud-BrandonJones Sep 29, 2017
bf485af
Merge branch 'feature-help-modal-header' of https://github.com/Blackb…
Blackbaud-BrandonJones Sep 29, 2017
76f33f2
Merge branch 'master' of github.com:blackbaud/skyux2 into feature-hel…
Blackbaud-BrandonJones Sep 29, 2017
bcf506e
removed extra space
Blackbaud-BrandonJones Sep 29, 2017
9ea609c
Merge branch 'master' into feature-help-modal-header
Blackbaud-BrandonJones Sep 29, 2017
12eb50d
Merge branch 'master' into feature-help-modal-header
Blackbaud-BrandonJones Oct 3, 2017
5e11169
Merge branch 'master' of github.com:blackbaud/skyux2 into feature-hel…
Blackbaud-BrandonJones Oct 3, 2017
a672032
Merge branch 'master' into feature-help-modal-header
Blackbaud-BrandonJones Oct 3, 2017
705a9a6
Merge branch 'feature-help-modal-header' of https://github.com/Blackb…
Blackbaud-BrandonJones Oct 3, 2017
971e631
responding to feedback
Blackbaud-BrandonJones Oct 4, 2017
3c72e95
Merge branch 'master' of github.com:blackbaud/skyux2 into feature-hel…
Blackbaud-BrandonJones Oct 4, 2017
93217ce
unsubscribing
Blackbaud-BrandonJones Oct 4, 2017
5e5f19b
Merge branch 'master' into feature-help-modal-header
Blackbaud-SteveBrush Oct 4, 2017
81b3870
unsubbing
Blackbaud-BrandonJones Oct 4, 2017
ae134e7
Merge branch 'feature-help-modal-header' of https://github.com/Blackb…
Blackbaud-BrandonJones Oct 4, 2017
1ac1dfa
removed unsub stuff
Blackbaud-BrandonJones Oct 4, 2017
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,14 @@
>
Open modal
</button>

<button
type="button"
class="sky-btn sky-btn-primary sky-modal-with-help"
(click)="openModalWithHelp()">
Open modal with Help
</button>

<button
type="button"
class="sky-btn sky-btn-primary sky-test-large-modal"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,17 @@ export class ModalVisualComponent {
constructor(private modal: SkyModalService) { }

public openModal() {

this.modal.open(ModalDemoComponent, { 'providers': [] });
}

public openModalWithHelp() {
this.modal.open(ModalDemoComponent, { 'providers': [], 'helpKey': 'demo-key.html' });
}

public openLargeModal() {
this.modal.open(ModalLargeDemoComponent, { 'providers': [] });
}

public openFullScreenModal() {
this.modal.open(ModalFullPageDemoComponent, { 'providers': [], 'fullPage': true });
}
Expand Down
32 changes: 32 additions & 0 deletions skyux-spa-visual-tests/src/app/modal/modal.visual-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,38 @@ describe('Modal', () => {

});

it('should match previous modal screenshot with help button in header', () => {
return SkyVisualTest.setupTest('modal')
.then(() => {
element(by.css('.sky-modal-with-help')).click();
SkyVisualTest.moveCursorOffScreen();
return SkyVisualTest.compareScreenshot({
screenshotName: 'modal-with-help',
selector: '.sky-modal',
checkAccessibility: true
}).then(() => {
element(by.css('.sky-modal .sky-modal-btn-close')).click();
});
});

});

it('should match previous modal screenshot with help button in header on small screens', () => {
return SkyVisualTest.setupTest('modal', 480)
.then(() => {
element(by.css('.sky-modal-with-help')).click();
SkyVisualTest.moveCursorOffScreen();
return SkyVisualTest.compareScreenshot({
screenshotName: 'modal-with-help-small',
selector: '.sky-modal',
checkAccessibility: true
}).then(() => {
element(by.css('.sky-modal .sky-modal-btn-close')).click();
});
});

});

it('should match previous modal screenshot on small screens', () => {
return SkyVisualTest.setupTest('modal', 480)
.then(() => {
Expand Down
30 changes: 21 additions & 9 deletions src/app/components/modal/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -8,34 +8,34 @@
<sky-demo-page-property
propertyName="open(component, config?)"
>
Opens a modal using the specified component. The optional <code>config</code> parameter is an object literal to be passed to the component's constructor.This method returns the modal instance.
Opens a modal using the specified component. The optional <code>config</code> parameter is an object literal to be passed to the component's constructor. This method returns the modal instance.
</sky-demo-page-property>
</sky-demo-page-properties>

<sky-demo-page-properties sectionHeading="Modal service config properties">
<sky-demo-page-property
propertyName="fullPage?"
propertyName="fullPage"
>
A Boolean property of <code>config</code> with a default of false. When set to true, the modal will become full screen.
</sky-demo-page-property>

<sky-demo-page-property
propertyName="tiledBody?"
propertyName="tiledBody"
defaultValue="false"
isOptional="true"
>
Indicates whether the modal uses tiles. When set to true, the modal's background switches to <code>$sky-background-color-neutral-light</code> and tile headings are styled as subsection headings.
Indicates whether the modal uses tiles. When set to true, the modal's background switches to <code>$sky-background-color-neutral-light</code> and tile headings are styled as subsection headings.
</sky-demo-page-property>

<sky-demo-page-property
propertyName="providers?"
propertyName="providers"
>
A array property of <code>providers</code> This can be used to pass context values from the component launching the modal to the modal component.

</sky-demo-page-property>

<sky-demo-page-property
propertyName="size?"
propertyName="size"
defaultValue="medium"
isOptional="true"
>
Expand All @@ -44,16 +44,22 @@
</sky-demo-page-property>

<sky-demo-page-property
propertyName="ariaLabelledBy?"
propertyName="ariaLabelledBy"
isOptional="true"
>
Sets the <code>aria-labelledby</code> attribute for the modal dialog for accessibility support. The value should be an id (without the leading <code>#</code>) pointing to the element that labels your modal. Typically, this will be a header element. If not provided, this will default to the content of the <code>sky-modal-header</code> component.
</sky-demo-page-property>
<sky-demo-page-property
propertyName="ariaDescribedBy?"
propertyName="ariaDescribedBy"
isOptional="true"
>
Sets the <code>aria-describedby</code> attribute for the modal dialog for accessibility support. The value should be an id (without the leading <code>#</code>) pointing to the element that describes your modal. Typically, this will be the text on your modal, but does not include something the user would interact with, like buttons or a form. If not provided, this will default to the content of the <code>sky-modal-content</code> component.
Sets the <code>aria-describedby</code> attribute for the modal dialog for accessibility support. The value should be an id (without the leading <code>#</code>) pointing to the element that describes your modal. Typically, this will be the text on your modal, but does not include something the user would interact with, like buttons or a form. If not provided, this will default to the content of the <code>sky-modal-content</code> component.
</sky-demo-page-property>

<sky-demo-page-property
propertyName="helpKey"
isOptional="true">
A string that specifies a <code>helpKey</code>. Setting this attribute causes the <i class="fa fa-question-circle" aria-hidden="true"></i> button to show in the modal header. When this button is clicked the <code>helpInvoked</code> event is is broadcast with the helpKey.
</sky-demo-page-property>
</sky-demo-page-properties>

Expand Down Expand Up @@ -95,6 +101,12 @@
propertyName="closed">
An event that is emitted by the modal instance when it is closed. It emits a <code>SkyCloseModalArgs</code> object that has a <code>data</code> property with data passed from the user on close or save and a <code>reason</code> property which indicates whether the modal was saved or closed without saving.
</sky-demo-page-property>

<sky-demo-page-property
propertyName="helpInvoked"
isOptional="true">
When the <code>helpKey</code> attribute is defined and the <i class="fa fa-question-circle" aria-hidden="true"></i> is clicked, the <code>helpInvoked</code> event is emitted by the modal instance. It emits the specified <code>helpKey</code> string.
</sky-demo-page-property>
</sky-demo-page-properties>

<sky-demo-page-example>
Expand Down
1 change: 1 addition & 0 deletions src/app/components/modal/modal-demo-context.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
export class SkyModalDemoContext {
public valueA: string;
public eventMessage?: string;
}
5 changes: 5 additions & 0 deletions src/app/components/modal/modal-demo-form.component.html
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,11 @@
Value A
<input type="text" [(ngModel)]="context.valueA">
</label>
<p>
<sky-alert *ngIf="context.eventMessage">
{{context.eventMessage}}
</sky-alert>
</p>
</sky-modal-content>
<sky-modal-footer>
<button
Expand Down
6 changes: 5 additions & 1 deletion src/app/components/modal/modal-demo.component.html
Original file line number Diff line number Diff line change
Expand Up @@ -16,4 +16,8 @@

<button type="button" class="sky-btn sky-btn-primary" (click)="openModal('tiledModal')">
Open tiled modal
</button>
</button>

<button type="button" class="sky-btn sky-btn-primary" (click)="openModal('withHelpHeader')">
Open modal with help header
</button>
9 changes: 9 additions & 0 deletions src/app/components/modal/modal-demo.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,9 @@ export class SkyModalDemoComponent {
case 'tiledModal':
modalInstanceType = SkyModalDemoTiledFormComponent;
break;
case 'withHelpHeader':
options.helpKey = 'demo-key.html';
break;
default:
break;
}
Expand All @@ -46,5 +49,11 @@ export class SkyModalDemoComponent {
modalInstance.closed.first().subscribe((result: SkyModalCloseArgs) => {
console.log(`Modal closed with reason: ${result.reason} and data: ${result.data}`);
});

modalInstance.helpInvoked.subscribe((helpKey: string) => {
context.eventMessage = `
Modal header help was invoked with the following help key: ${helpKey}
`;
});
}
}
4 changes: 4 additions & 0 deletions src/locales/resources_en_US.json
Original file line number Diff line number Diff line change
Expand Up @@ -599,6 +599,10 @@
"_description": "Text for modal close button",
"message": "Close modal"
},
"open_help": {
"_description": "Text for modal header Open Help button",
"message": "Open Help"
},
"modal_footer_cancel_button": {
"_description": "Default lable text for modal cancel button",
"message": "Cancel"
Expand Down
1 change: 1 addition & 0 deletions src/modules/modal/modal-configuration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ export class SkyModalConfiguration {
public ariaDescribedBy?: string;
public ariaLabelledBy?: string;
public tiledBody?: boolean;
public helpKey?: string;

constructor() {
this.fullPage = this.fullPage;
Expand Down
4 changes: 4 additions & 0 deletions src/modules/modal/modal-host.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,10 @@ export class SkyModalHostComponent {
modalComponentRef.destroy();
}

hostService.openHelp.subscribe((helpKey?: string) => {

Choose a reason for hiding this comment

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

I just noticed that none of these subscriptions are unsubscribed during ngOnDestroy. Do you mind adding this to the component to see if it works?

modalInstance.openHelp(helpKey);
});

hostService.close.subscribe(() => {
modalInstance.close();
});
Expand Down
20 changes: 20 additions & 0 deletions src/modules/modal/modal-host.service.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,4 +61,24 @@ describe('Modal host service', () => {
expect(closeEmitted).toBe(true);
service.destroy();
});

it('should notify subscribers when the help header button is clicked', () => {
const testHelpKey = 'test-key.html';
let helpKey = '';
let helpClicked = false;

let service = new SkyModalHostService();

service.openHelp.subscribe((key: string) => {
helpClicked = true;
helpKey = key;
});

service.onOpenHelp(testHelpKey);

expect(helpClicked).toBe(true);
expect(helpKey).toBe(testHelpKey);

service.destroy();
});
});
5 changes: 5 additions & 0 deletions src/modules/modal/modal-host.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ export class SkyModalHostService {
private static modalHosts: SkyModalHostService[] = [];

public close = new EventEmitter<void>();
public openHelp = new EventEmitter<any>();

public constructor() {
SkyModalHostService.modalHosts.push(this);
Expand All @@ -39,6 +40,10 @@ export class SkyModalHostService {
this.close.emit();
}

public onOpenHelp(helpKey?: string) {
this.openHelp.emit(helpKey);
}

public destroy(): void {
SkyModalHostService.modalHosts.splice(SkyModalHostService.modalHosts.indexOf(this));
}
Expand Down
5 changes: 5 additions & 0 deletions src/modules/modal/modal-instance.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ export class SkyModalInstance {
public componentInstance: any;

public closed = new EventEmitter<SkyModalCloseArgs>();
public helpInvoked = new EventEmitter<any>();

public close(result?: any, reason?: string) {
if (reason === undefined) {
Expand All @@ -27,6 +28,10 @@ export class SkyModalInstance {
this.closeModal('save', result);
}

public openHelp(helpKey?: string) {
this.helpInvoked.emit(helpKey);

Choose a reason for hiding this comment

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

I'd prefer the public method's name was more similar to the emitter (this.helpOpened, or vice versa).

}

private closeModal(type: string, result?: any) {
const args = new SkyModalCloseArgs();

Expand Down
3 changes: 3 additions & 0 deletions src/modules/modal/modal.component.html
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,9 @@
<ng-content select="sky-modal-header"></ng-content>
</div>
<div class="sky-modal-header-buttons">
<button *ngIf="helpKey !== undefined" type="button" class="sky-btn" name="help-button" [attr.aria-label]="'open_help' | skyResources" (click)="helpButtonClick()">

Choose a reason for hiding this comment

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

Is the !== undefined needed here? Isn't *ngIf="helpKey" sufficient?

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 should be sufficient, the difference would be that ngIf would be getting a direct boolean value by evaluating the expression, instead of an interpolated undefined vs something exists check. I am fine with either :) depends on your standards willing to adjust.

<i class="fa fa-question-circle"></i>
</button>

<button type="button" class="sky-btn sky-modal-btn-close" [attr.aria-label]="'modal_close' | skyResources" (click)="closeButtonClick()">

Expand Down
15 changes: 15 additions & 0 deletions src/modules/modal/modal.component.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,21 @@ describe('Modal component', () => {
applicationRef.tick();
}));

it('should trigger the help modal when the help button is clicked', fakeAsync(() => {
let modalInstance = openModal(ModalTestComponent, { helpKey: 'default.html' });
spyOn(modalInstance, 'openHelp').and.callThrough();

expect(document.querySelector('.sky-modal')).toExist();

(<any>document.querySelector('button[name="help-button"]')).click();

expect(modalInstance.openHelp).toHaveBeenCalledWith('default.html');

applicationRef.tick();

closeModal(modalInstance);
}));

it('should set max height based on window and change when window resizes', fakeAsync(() => {
let modalInstance = openModal(ModalTestComponent);
let modalEl = document.querySelector('.sky-modal');
Expand Down
8 changes: 8 additions & 0 deletions src/modules/modal/modal.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,10 @@ export class SkyModalComponent implements AfterViewInit {
return this.config.ariaLabelledBy || this.modalHeaderId;
}

public get helpKey() {
return this.config.helpKey;
}

@HostListener('document:keydown', ['$event'])
public onDocumentKeyDown(event: KeyboardEvent) {
/* istanbul ignore else */
Expand Down Expand Up @@ -145,6 +149,10 @@ export class SkyModalComponent implements AfterViewInit {
});
}

public helpButtonClick() {
this.hostService.onOpenHelp(this.helpKey);
}

public closeButtonClick() {
this.hostService.onClose();
}
Expand Down
1 change: 1 addition & 0 deletions src/modules/modal/modal.interface.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,5 @@ export interface SkyModalConfigurationInterface {
ariaDescribedBy?: string;
ariaLabelledBy?: string;
tiledBody?: boolean;
helpKey?: string;
}