feat(annotations): Don't show highlight button if no download permission #1253
Conversation
ConradJChan
changed the title
| @@ -1105,11 +1105,13 @@ class DocBaseViewer extends BaseViewer { | |||
| this.controls.add(__('exit_fullscreen'), this.toggleFullscreen, 'bp-exit-fullscreen-icon', ICON_FULLSCREEN_OUT); | |||
|
|
|||
| if (this.areNewAnnotationsEnabled() && this.hasAnnotationCreatePermission()) { | |||
| const canDownload = checkPermission(this.options.file, PERMISSION_DOWNLOAD); | |||
There was a problem hiding this comment.
Should we use the canDownload util from lib/file.js?
There was a problem hiding this comment.
Seems like canDownload from lib/file.js checks a lot more than just the download permission related to actually being able to download the file. Might be too restrictive?
There was a problem hiding this comment.
Yeah, fair point. It also looks like we only check the permission for textLayerMode. Should be good to go here.
| @@ -2380,6 +2381,18 @@ describe('src/lib/viewers/doc/DocBaseViewer', () => { | |||
| }), | |||
| ); | |||
|
|
|||
| it('should not showHighlightText if file has no download permission', () => { | |||
| stubs.checkPermission.withArgs(docBase.options.file, PERMISSION_DOWNLOAD).returns(false); | |||
There was a problem hiding this comment.
Any way we can avoid using withArgs here? Will make converting to Jest (someday) easier.
There was a problem hiding this comment.
I looked into this but it turns out stubs.checkPermission is already being used in the other tests
| @@ -1105,11 +1105,13 @@ class DocBaseViewer extends BaseViewer { | |||
| this.controls.add(__('exit_fullscreen'), this.toggleFullscreen, 'bp-exit-fullscreen-icon', ICON_FULLSCREEN_OUT); | |||
|
|
|||
| if (this.areNewAnnotationsEnabled() && this.hasAnnotationCreatePermission()) { | |||
| const canDownload = checkPermission(this.options.file, PERMISSION_DOWNLOAD); | |||
There was a problem hiding this comment.
Yeah, fair point. It also looks like we only check the permission for textLayerMode. Should be good to go here.
No description provided.