-
Notifications
You must be signed in to change notification settings - Fork 116
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
feat(viewer): Add support for disabling .boxdicom file type #1436
Changes from 1 commit
35bd0b8
de2dd56
6b9ff39
abc58ae
314cbcd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,4 @@ | ||
import getProp from 'lodash/get'; | ||
import AssetLoader from '../AssetLoader'; | ||
import IFrameViewer from './IFrameViewer'; | ||
import { ORIGINAL_REP_NAME } from '../../constants'; | ||
|
@@ -22,6 +23,20 @@ class IFrameLoader extends AssetLoader { | |
super(); | ||
this.viewers = VIEWERS; | ||
} | ||
|
||
/** | ||
* @inheritdoc | ||
*/ | ||
determineViewer(file, disabledViewers = [], viewerOptions = {}) { | ||
const isDicomFile = file.name === 'Dicom.boxdicom' || file.extension === 'boxdicom'; | ||
const openWithAmbraEnabled = getProp(viewerOptions, 'IFrame.openWithAmbra'); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Instead of openWithAmbra, maybe the option should be |
||
// The IFrame viewer is disabled when the file is a Boxdicom file and Open_with_Ambra FF is enabled | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We shouldn't mention FF from other codebases here |
||
if (openWithAmbraEnabled && isDicomFile) { | ||
disabledViewers.push('IFrame'); | ||
} | ||
|
||
return super.determineViewer(file, disabledViewers); | ||
jstoffan marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
} | ||
|
||
export default new IFrameLoader(); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,6 @@ | ||
import IFrameViewer from '../IFrameViewer'; | ||
import BaseViewer from '../../BaseViewer'; | ||
import IFrameLoader from '../IFrameLoader'; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sort the imports |
||
|
||
let containerEl; | ||
let iframe; | ||
|
@@ -15,6 +16,13 @@ describe('lib/viewers/iframe/IFrameViewer', () => { | |
file: { | ||
id: '123', | ||
extension: 'boxnote', | ||
representations: { | ||
entries: [ | ||
{ | ||
representation: 'ORIGINAL', | ||
}, | ||
], | ||
}, | ||
}, | ||
}); | ||
|
||
|
@@ -82,6 +90,42 @@ describe('lib/viewers/iframe/IFrameViewer', () => { | |
iframe.load(); | ||
}); | ||
|
||
test('should not return a viewer if file is a boxdicom and the open_with_ambra FF is enabled', () => { | ||
iframe.options.file.extension = 'boxdicom'; | ||
|
||
const viewerOptions = { | ||
IFrame: { | ||
openWithAmbra: true, | ||
}, | ||
}; | ||
const viewer = IFrameLoader.determineViewer(iframe, [], viewerOptions); | ||
expect(viewer).toBeUndefined(); | ||
}); | ||
|
||
test('should return a viewer if file is a boxnote and open_with_ambra FF is enabled', () => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could lump these together with a |
||
iframe.options.file.extension = 'boxnote'; | ||
|
||
const viewerOptions = { | ||
IFrame: { | ||
openWithAmbra: true, | ||
}, | ||
}; | ||
const viewer = IFrameLoader.determineViewer(iframe.options.file, [], viewerOptions); | ||
expect(viewer).toBeDefined(); | ||
}); | ||
|
||
test('should return a viewer if open_with_ambra FF is disabled', () => { | ||
iframe.options.file.extension = 'boxdicom'; | ||
|
||
const viewerOptions = { | ||
IFrame: { | ||
openWithAmbra: false, | ||
}, | ||
}; | ||
const viewer = IFrameLoader.determineViewer(iframe.options.file, [], viewerOptions); | ||
expect(viewer).toBeDefined(); | ||
}); | ||
|
||
test('should invoke startLoadTimer()', () => { | ||
const stub = jest.spyOn(iframe, 'startLoadTimer'); | ||
iframe.load(); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The extension check is probably sufficient here