Skip to content
This repository has been archived by the owner on May 7, 2021. It is now read-only.

Commit

Permalink
fix(markdown): will allow or disallow saving empty field (#148)
Browse files Browse the repository at this point in the history 
* fix(bug): replaced nodeValue with childNodes

- The value of nodeValue is coming null sometime
- To reproduce the bug start editing, blur the field and focus again on the field and see th error on console.

* feat(markdown): added option to allow or disallow saving empty field

* fix(markdown): tests added to verify allowEmptyField flag
  • Loading branch information
@sanbornsen @joshuawilson
sanbornsen authored and joshuawilson committed Jun 12, 2018
1 parent 7dcc8a1 commit b378247
Show file tree
Hide file tree
Showing 5 changed files with 134 additions and 78 deletions.
10 changes: 5 additions & 5 deletions src/app/editable/almeditable.directive.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,12 @@ export class AlmEditableDirective implements OnInit, OnChanges {
@Output('onUpdate') onUpdate = new EventEmitter();
@Input() editable = true;

constructor(private elementRef: ElementRef) {
}

private content: any = '';
private element: HTMLElement = this.elementRef.nativeElement;

constructor(private elementRef: ElementRef) {
}

ngOnInit() {
this.element.style.whiteSpace = 'pre-wrap';
if (this.editable) {
Expand All @@ -42,7 +42,7 @@ export class AlmEditableDirective implements OnInit, OnChanges {

onEdit() {
let newContent = this.element.innerText;
if (this.content != newContent) {
if (this.content !== newContent) {
this.content = newContent;
this.onUpdate.emit(this.content);
}
Expand Down Expand Up @@ -73,7 +73,7 @@ export class AlmEditableDirective implements OnInit, OnChanges {
let sel = window.getSelection();
range.setStart(
this.element.childNodes[this.element.childNodes.length - 1],
this.element.childNodes[this.element.childNodes.length - 1].nodeValue.length
this.element.childNodes[this.element.childNodes.length - 1].childNodes.length
);
range.collapse(true);
sel.removeAllRanges();
Expand Down
14 changes: 14 additions & 0 deletions src/app/markdown/examples/markdown-example.component.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,20 @@
</div>
</div>

<label> Field with restricted empty field save </label>
<div class="row">
<div class="col-xs-12">
<f8-markdown
[rawText]="''"
[renderedText]="''"
[placeholder]="'This is placeholder text'"
[allowEmptySave]="false"
(onSaveClick)="onSaveOrPreview($event)"
(showPreview)="onSaveOrPreview($event)">
</f8-markdown>
</div>
</div>

<label> Field with placeholder </label>
<div class="row">
<div class="col-xs-12">
Expand Down
4 changes: 2 additions & 2 deletions src/app/markdown/markdown.component.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
*ngIf="viewType==='markdown'"
[editable]="true"
class="editor-box editor-markdown"
(keyup)="editorKeyUp($event)"
[innerText]="rawText">
</p>
<div #editorBox #previewArea
Expand Down Expand Up @@ -53,8 +54,7 @@
<button
(click)="saveClick()"
class="fl btn btn-primary pull-right action-btn btn-save"
[disabled]="saving"
[class.disabled]="saving">
[disabled]="saving || (!allowEmptySave && fieldEmpty)">
<i class="fa fa-check"></i>
</button>
</div>
140 changes: 86 additions & 54 deletions src/app/markdown/markdown.component.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,58 +30,90 @@ describe('Markdown component - ', () => {
});
}));

it('Should handle Markdown checkboxes correctly.',
inject([DomSanitizer], (domSanitizer: DomSanitizer) => {
// tslint:disable-next-line:max-line-length
comp.inpRawText = '# hello, markdown!\n* [ ] Item 1\n* [x] Item 2\n* [ ] Item 3';
let originalHTML = '<h1>hello, markdown!\</h1><ul>' +
// tslint:disable-next-line:max-line-length
'<li><input class="markdown-checkbox" type="checkbox" data-checkbox-index="0"></input> Item 0</li>' +
// tslint:disable-next-line:max-line-length
'<li><input class="markdown-checkbox" type="checkbox" checked="" data-checkbox-index="1"></input> Item 1</li>' +
// tslint:disable-next-line:max-line-length
'<li><input class="markdown-checkbox" type="checkbox" data-checkbox-index="2"></input> Item 2</li></ul>';
// in this test, we provide a SaveValue to the component.
comp.inpRenderedText = domSanitizer.bypassSecurityTrustHtml(originalHTML);
// this first detectChanges() updates the component that one of the @Inputs has changed.
fixture.detectChanges();
// because of https://github.com/angular/angular/issues/9866, detectChanges() does not
// call ngOnChanges() on the component (yeah, it it as broken as it sounds). So
// we need to call the component manually to update.
comp.ngOnChanges({
inpRawText: {} as SimpleChange,
inpRenderedText: {} as SimpleChange
} as SimpleChanges);
// and because the test framework is not even able to detect inner changes to a component,
// we need to call detectChanges() again.
fixture.detectChanges();
// also, using query() is also not working. Maybe due to the dynamic update of innerHTML.
// So we need to use the nativeElement to get a selector working.
// tslint:disable-next-line:max-line-length
let markdownPreview: Element = fixture.debugElement.nativeElement.querySelector('.markdown-rendered');
expect(markdownPreview).not.toBeNull();
// preview render of the template default
let markdownCheckboxElementList = markdownPreview.querySelectorAll('.markdown-checkbox');
expect(markdownCheckboxElementList).not.toBeNull();
expect(markdownCheckboxElementList.length).toBe(3);
expect(markdownCheckboxElementList[0].hasAttribute('checked')).toBeFalsy();
expect(markdownCheckboxElementList[1].hasAttribute('checked')).toBeTruthy();
expect(markdownCheckboxElementList[2].hasAttribute('checked')).toBeFalsy();
// tick a checkbox
let checkboxElem = markdownCheckboxElementList[0] as HTMLElement;
checkboxElem.click();
// see if it ends up in the Markdown
expect(comp.rawText.indexOf('[x] Item 1')).toBeGreaterThan(-1);
// tick another checkbox
checkboxElem = markdownCheckboxElementList[2] as HTMLElement;
checkboxElem.click();
// see if it ends up in the Markdown
expect(comp.rawText.indexOf('[x] Item 3')).toBeGreaterThan(-1);
// untick a checkbox
checkboxElem = markdownCheckboxElementList[1] as HTMLElement;
checkboxElem.click();
// see if it ends up in the Markdown
expect(comp.rawText.indexOf('[ ] Item 2')).toBeGreaterThan(-1);
})
);
it('Should handle Markdown checkboxes correctly.',
inject([DomSanitizer], (domSanitizer: DomSanitizer) => {
// tslint:disable-next-line:max-line-length
comp.inpRawText = '# hello, markdown!\n* [ ] Item 1\n* [x] Item 2\n* [ ] Item 3';
let originalHTML = '<h1>hello, markdown!\</h1><ul>' +
// tslint:disable-next-line:max-line-length
'<li><input class="markdown-checkbox" type="checkbox" data-checkbox-index="0"></input> Item 0</li>' +
// tslint:disable-next-line:max-line-length
'<li><input class="markdown-checkbox" type="checkbox" checked="" data-checkbox-index="1"></input> Item 1</li>' +
// tslint:disable-next-line:max-line-length
'<li><input class="markdown-checkbox" type="checkbox" data-checkbox-index="2"></input> Item 2</li></ul>';
// in this test, we provide a SaveValue to the component.
comp.inpRenderedText = domSanitizer.bypassSecurityTrustHtml(originalHTML);
// this first detectChanges() updates the component that one of the @Inputs has changed.
fixture.detectChanges();
// because of https://github.com/angular/angular/issues/9866, detectChanges() does not
// call ngOnChanges() on the component (yeah, it it as broken as it sounds). So
// we need to call the component manually to update.
comp.ngOnChanges({
inpRawText: {} as SimpleChange,
inpRenderedText: {} as SimpleChange
} as SimpleChanges);
// and because the test framework is not even able to detect inner changes to a component,
// we need to call detectChanges() again.
fixture.detectChanges();
// also, using query() is also not working. Maybe due to the dynamic update of innerHTML.
// So we need to use the nativeElement to get a selector working.
// tslint:disable-next-line:max-line-length
let markdownPreview: Element = fixture.debugElement.nativeElement.querySelector('.markdown-rendered');
expect(markdownPreview).not.toBeNull();
// preview render of the template default
let markdownCheckboxElementList = markdownPreview.querySelectorAll('.markdown-checkbox');
expect(markdownCheckboxElementList).not.toBeNull();
expect(markdownCheckboxElementList.length).toBe(3);
expect(markdownCheckboxElementList[0].hasAttribute('checked')).toBeFalsy();
expect(markdownCheckboxElementList[1].hasAttribute('checked')).toBeTruthy();
expect(markdownCheckboxElementList[2].hasAttribute('checked')).toBeFalsy();
// tick a checkbox
let checkboxElem = markdownCheckboxElementList[0] as HTMLElement;
checkboxElem.click();
// see if it ends up in the Markdown
expect(comp.rawText.indexOf('[x] Item 1')).toBeGreaterThan(-1);
// tick another checkbox
checkboxElem = markdownCheckboxElementList[2] as HTMLElement;
checkboxElem.click();
// see if it ends up in the Markdown
expect(comp.rawText.indexOf('[x] Item 3')).toBeGreaterThan(-1);
// untick a checkbox
checkboxElem = markdownCheckboxElementList[1] as HTMLElement;
checkboxElem.click();
// see if it ends up in the Markdown
expect(comp.rawText.indexOf('[ ] Item 2')).toBeGreaterThan(-1);
})
);

fit('should emit output onsave empty field when ' +
'`allowEmptySave` is false and the field is empty', () => {
spyOn(comp.onSaveClick, 'emit');
comp.allowEmptySave = false;
comp.fieldEmpty = true;
comp.previousRawText = 'abc';
comp.rawText = 'xyz';
comp.saveClick();
expect(comp.onSaveClick.emit).not.toHaveBeenCalled();
});

fit('should emit output onsave empty field when `allowEmptySave` is true', () => {
spyOn(comp.onSaveClick, 'emit');
comp.allowEmptySave = true;
comp.fieldEmpty = true;
comp.previousRawText = 'abc';
comp.rawText = 'xyz';
comp.saveClick();
expect(comp.onSaveClick.emit).toHaveBeenCalled();
});

fit('should emit output onsave empty field when ' +
'`allowEmptySave` is false and the field is not empty', () => {
spyOn(comp.onSaveClick, 'emit');
comp.allowEmptySave = false;
comp.fieldEmpty = false;
comp.previousRawText = 'abc';
comp.rawText = 'xyz';
comp.saveClick();
expect(comp.onSaveClick.emit).toHaveBeenCalled();
});
});
44 changes: 27 additions & 17 deletions src/app/markdown/markdown.component.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ export class MarkdownComponent implements OnChanges, OnInit, AfterViewChecked {
@Input() placeholder: string = 'This is place holder';
@Input() editAllow: boolean = true;
@Input() renderedHeight: number = 300;
@Input() allowEmptySave: boolean = true;

@Output() onActiveEditor = new EventEmitter();
@Output() onSaveClick = new EventEmitter();
Expand All @@ -54,14 +55,16 @@ export class MarkdownComponent implements OnChanges, OnInit, AfterViewChecked {
// these need to be public for the tests accessing them.
renderedText: any = '';
rawText = '';
fieldEmpty: boolean = true;
previousRawText = '';

private markdownViewExpanded: boolean = false;
private tabBarVisible: boolean = true;
private viewType: string = 'preview'; // markdown
private editorActive: boolean = false;
private showMore = false;
private inputsDisabled = false;

private previousRawText = '';
private previousRenderedText = '';

constructor(private cdr: ChangeDetectorRef, private elementRef: ElementRef) {}
Expand Down Expand Up @@ -244,25 +247,32 @@ export class MarkdownComponent implements OnChanges, OnInit, AfterViewChecked {
}

saveClick() {
if (this.viewType === 'markdown' &&
this.previousRawText !== this.editorInput.nativeElement.innerText.trim()) {
this.saving = true;
this.onSaveClick.emit({
rawText: this.editorInput.nativeElement.innerText.trim(),
callBack: (t: string, m: string) => this.saveUpdate(t, m)
});
} else if (this.viewType === 'preview' &&
this.previousRawText !== this.rawText) {
this.saving = true;
this.onSaveClick.emit({
rawText: this.rawText,
callBack: (t: string, m: string) => this.saveUpdate(t, m)
});
} else {
this.deactivateEditor();
if (this.allowEmptySave || (!this.fieldEmpty && !this.allowEmptySave)) {
if (this.viewType === 'markdown' &&
this.previousRawText !== this.editorInput.nativeElement.innerText.trim()) {
this.saving = true;
this.onSaveClick.emit({
rawText: this.editorInput.nativeElement.innerText.trim(),
callBack: (t: string, m: string) => this.saveUpdate(t, m)
});
} else if (this.viewType === 'preview' &&
this.previousRawText !== this.rawText) {
this.saving = true;
this.onSaveClick.emit({
rawText: this.rawText,
callBack: (t: string, m: string) => this.saveUpdate(t, m)
});
} else {
this.deactivateEditor();
}
}
}

editorKeyUp(event: Event) {
this.fieldEmpty =
event.srcElement.textContent.trim() === '';
}

closeClick() {
// Restore saved previous data
this.rawText = this.previousRawText;
Expand Down

0 comments on commit b378247

Please sign in to comment.