Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Modifications for latest material design and rxjs #1

Closed
gordonmckenzie opened this issue Jul 29, 2018 · 6 comments
Closed

Modifications for latest material design and rxjs #1

gordonmckenzie opened this issue Jul 29, 2018 · 6 comments

Comments

@gordonmckenzie
Copy link

Absolutely fantastic effort RichonChen, really well done! I've been looking for something as true to Google's own version for some time and an Angular 4+ version with Angular Material is perfect!

I have made some modifications to get this working in Angular 6, rxjs version 6+ and the latest Angular Material. It seems to work really well now.

I'm new to GitHub, but these changes may be useful to others: -

  1. All references to Angular Material objects should be prefixed Mat instead of Md for imports and mat- instead of md- for HTML elements. For example in feedback.module.ts change the following: -

import { MatDialogModule, MatButtonModule, MatIconModule, MatInputModule, MatTooltipModule, MatCheckboxModule, MatProgressSpinnerModule } from '@angular/material';

imports: [ MatDialogModule, MatButtonModule, MatIconModule, MatInputModule, MatTooltipModule, CommonModule, FormsModule, MatCheckboxModule, MatProgressSpinnerModule ]

  1. Update to use the most recent version of rxjs.

For example in feedback.service.ts...
import { Subject } from 'rxjs';

In feedback-dialog.component.ts...

import { Observable, fromEvent, from } from 'rxjs'; import {mergeMap, map, finalize, takeUntil} from 'rxjs/operators';

and change the use of the operators to a pipe rather than chaining, as follows: -

return mouseMove.pipe( map((mouseMoveEvent: MouseEvent) => { newRectangle.width = mouseMoveEvent.clientX - mouseDownEvent.clientX; newRectangle.height = mouseMoveEvent.clientY - mouseDownEvent.clientY; return newRectangle; }), finalize(() => { this.rectangles.push(newRectangle); }), takeUntil(mouseUp));

similarly in feedback-toolbar.component.ts...

return mouseMove.pipe( map((mm: MouseEvent) => { mm.preventDefault(); return { left: mm.clientX - startX, top: mm.clientY - startY }; }), finalize(() => { this.disableToolbarTips = false; }), takeUntil(mouseUp)); }));

  1. By far the most important change is to update the return from html2canvas to be a promise like this: -

html2canvas(wholehtml).then(canvas => { this.screenshotCanvasSource.next(canvas); });

Once again, great effort, well done.

@gordonmckenzie
Copy link
Author

Further to the above, there is an issue regarding the final merge of the screenshotCanvas and the mergedCanvas. It might not be the prettiest solution, but I managed to get this to work using the following modification within feedback.dialog.component.ts: -

public mergeCanvas() { this.showToolbar = false; this.detector.detectChanges(); this.appendScreenshot(); let mergedCanvas = document.createElement('canvas'); let x; let y; let ctx = mergedCanvas.getContext('2d'); if (ctx === null) { return; } x = mergedCanvas.width; y = mergedCanvas.height; mergedCanvas.width = this.screenshotCanvas.width; mergedCanvas.height = this.screenshotCanvas.height; ctx.drawImage(this.screenshotCanvas, 0, 0); ctx.drawImage(this.drawCanvas, 0, 0); this.feedback.screenshot = mergedCanvas.toDataURL('image/png'); mergedCanvas.width = x; mergedCanvas.height = y; ctx.clearRect(0, 0, mergedCanvas.width, mergedCanvas.height); ctx.drawImage(this.screenshotCanvas, 0, 0, 360, 200); ctx.drawImage(this.drawCanvas, 0, 0, 360, 200); this.feedbackService.setCanvas(mergedCanvas); }

@RickonChen
Copy link
Owner

Hi gordonmckenzie
Thanks for your attention. I will take some time to review your advice and code. If there is no issue i will update the code asap. BTW, it is better if you can make pull request so that I could try your code directly. Thanks again!

@hakkurishian
Copy link

Hi gordonmckenzie
Thanks for your attention. I will take some time to review your advice and code. If there is no issue i will update the code asap. BTW, it is better if you can make pull request so that I could try your code directly. Thanks again!

@RickonChen did you manage to look into it? I'm planning to use this great module in my app using Angular 6.0. In case you didn't have the time yet and won't be able to integrate it in the latest version I'd try to implement @gordonmckenzie 's changes in a pull request.

@RickonChen
Copy link
Owner

Hi @hakkurishian , the project is being upgraded with the newest version of angular, rxjs, etc. It should be completed before this sunday. Thanks.

@RickonChen
Copy link
Owner

The project has been upgraded as advice of @gordonmckenzie . It could be running without issue. I will publish newest package to npm later. Thanks.

@RickonChen
Copy link
Owner

The ng-feedback has been uploaded to npm and find no issue after testing.
https://www.npmjs.com/package/ng-feedback

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants