feat(print): add check to see if printing is available #1177
Conversation
mickr
commented
Feb 28, 2020
mickr
commented
- Adds helper method to check if printing is available on the preview instance
* Adds helper method to check if printing is available on the preview instance
|
This will be used by the contentPreview to check whether a file type will support printing, there is a PR coming for that. |
src/lib/Preview.js
Outdated
| @@ -1299,7 +1309,7 @@ class Preview extends EventEmitter { | |||
| if (canDownload(this.file, this.options)) { | |||
| this.ui.showDownloadButton(this.download); | |||
|
|
|||
| if (checkFeature(this.viewer, 'print')) { | |||
| if (this.canPrint()) { | |||
There was a problem hiding this comment.
Probably move this outside the canDownload check to flatten the conditions?
src/lib/__tests__/Preview-test.js
Outdated
|
|
||
| it('should return true is file is downloadable and has printing feature', () => { | ||
| stubs.canDownload.returns(true); | ||
| stubs.checkFeature.withArgs(sinon.match.any, 'print').returns(true); |
There was a problem hiding this comment.
Try to avoid using withArgs if possible. It’s difficult to migrate to other test frameworks like Jest.
| * @return {boolean} | ||
| */ | ||
| canPrint() { | ||
| return !!canDownload(this.file, this.options) && checkFeature(this.viewer, 'print'); |
There was a problem hiding this comment.
Is the !! necessary here?
There was a problem hiding this comment.
Both return booleans, but figured to ensure it is a boolean in case those interfaces change, I don't believe it hurts anything.
There was a problem hiding this comment.
Seems like the !! should be omitted if it's not needed.
| * @return {boolean} | ||
| */ | ||
| canPrint() { | ||
| return !!canDownload(this.file, this.options) && checkFeature(this.viewer, 'print'); |
There was a problem hiding this comment.
Seems like the !! should be omitted if it's not needed.
