fix: download tool fixes & improvements (#1235)

* fix: download tool fixes & improvements

* fix filename error

* Create variable to track erros

* Fix small console error

* Fix all conflicts and merge changes from latest master with this PR's improvements

* Small improvement on select and textinput labels

* Add new icon for unlink

* Refactor on download image modal

* Adding loading screen

* Fix translation issue

* Fixing aspect Ratio and E2E tests

* Allow empty value without setting to 0

* Remove eslint comments

* Fixing typos

Co-authored-by: Gustavo André Lelis <galelis@gmail.com>
Co-authored-by: Danny Brown <danny.ri.brown@gmail.com>
This commit is contained in:
Rodrigo Antinarelli 2020-01-30 00:39:44 -03:00 committed by GitHub
parent 96a8e26786
commit b9574b6efc
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
11 changed files with 380 additions and 177 deletions

View File

@ -0,0 +1,14 @@
{
"emptyFilenameError": "The file name cannot be empty.",
"fileType": "File Type",
"filename": "File Name",
"formTitle": "Please specify the dimensions, filename, and desired type for the output image.",
"imageHeight": "Image height (px)",
"imagePreview": "Image Preview",
"imageWidth": "Image width (px)",
"keepAspectRatio": "Keep aspect ratio",
"loadingPreview": "Loading Image Preview...",
"minHeightError": "The minimum valid height is 100px.",
"minWidthError": "The minimum valid width is 100px.",
"showAnnotations": "Show Annotations"
}

View File

@ -7,6 +7,7 @@ import Header from './Header.json';
import MeasurementTable from './MeasurementTable.json';
import StudyList from './StudyList.json';
import UserPreferencesModal from './UserPreferencesModal.json';
import ViewportDownloadForm from './ViewportDownloadForm.json';
export default {
'en-US': {
@ -19,5 +20,6 @@ export default {
MeasurementTable,
StudyList,
UserPreferencesModal,
ViewportDownloadForm,
},
};

View File

@ -1,9 +1,16 @@
import React, { useEffect, useState, createRef } from 'react';
import React, {
useRef,
useCallback,
useEffect,
useState,
createRef,
} from 'react';
import PropTypes from 'prop-types';
import { useTranslation } from 'react-i18next';
import './ViewportDownloadForm.styl';
import { TextInput, Select } from '@ohif/ui';
import { TextInput, Select, Icon } from '@ohif/ui';
import classnames from 'classnames';
const FILE_TYPE_OPTIONS = [
{
@ -17,6 +24,7 @@ const FILE_TYPE_OPTIONS = [
];
const DEFAULT_FILENAME = 'image';
const REFRESH_VIEWPORT_TIMEOUT = 1000;
const ViewportDownloadForm = ({
activeViewport,
@ -32,7 +40,7 @@ const ViewportDownloadForm = ({
maximumSize,
canvasClass,
}) => {
const [t] = useTranslation('Buttons');
const [t] = useTranslation('ViewportDownloadForm');
const [filename, setFilename] = useState(DEFAULT_FILENAME);
const [fileType, setFileType] = useState('jpg');
@ -44,6 +52,12 @@ const ViewportDownloadForm = ({
const [showAnnotations, setShowAnnotations] = useState(true);
const [keepAspect, setKeepAspect] = useState(true);
const [aspectMultiplier, setAspectMultiplier] = useState({
width: 1,
height: 1,
});
const [viewportElement, setViewportElement] = useState();
const [viewportElementDimensions, setViewportElementDimensions] = useState({
width: defaultSize,
@ -62,41 +76,129 @@ const ViewportDownloadForm = ({
height: defaultSize,
});
// Cornerstone's `enable/disable`
useEffect(() => {
enableViewport(viewportElement);
const [error, setError] = useState({
width: false,
height: false,
filename: false,
});
return () => {
disableViewport(viewportElement);
};
}, [disableViewport, enableViewport, viewportElement]);
const hasError = Object.values(error).includes(true);
useEffect(() => {
const { width, height } = viewportElementDimensions;
const validSize = value => (value >= minimumSize ? value : minimumSize);
const loadAndUpdateViewports = async () => {
await loadImage(activeViewport, viewportElement, width, height);
toggleAnnotations(showAnnotations, viewportElement);
const refreshViewport = useRef(null);
const {
dataUrl,
width: viewportElementWidth,
height: viewportElementHeight,
} = await updateViewportPreview(
viewportElement,
downloadCanvas.ref.current,
fileType
const downloadImage = () => {
downloadBlob(
filename || DEFAULT_FILENAME,
fileType,
viewportElement,
downloadCanvas.ref.current
);
};
/**
* @param {object} event - Input change event
* @param {string} dimension - "height" | "width"
*/
const onDimensionsChange = (event, dimension) => {
const oppositeDimension = dimension === 'height' ? 'width' : 'height';
const sanitizedTargetValue = event.target.value.replace(/\D/, '');
const isEmpty = sanitizedTargetValue === '';
const newDimensions = { ...dimensions };
const updatedDimension = isEmpty
? ''
: Math.min(sanitizedTargetValue, maximumSize);
if (updatedDimension === dimensions[dimension]) {
return;
}
newDimensions[dimension] = updatedDimension;
if (keepAspect && newDimensions[oppositeDimension] !== '') {
newDimensions[oppositeDimension] = Math.round(
newDimensions[dimension] * aspectMultiplier[oppositeDimension]
);
}
setViewportPreview(state => ({
// In current code, keepAspect is always `true`
// And we always start w/ a square width/height
setDimensions(newDimensions);
// Only update if value is non-empty
if (!isEmpty) {
setViewportElementDimensions(newDimensions);
setDownloadCanvas(state => ({
...state,
src: dataUrl,
width: validSize(viewportElementWidth),
height: validSize(viewportElementHeight),
...newDimensions,
}));
}
};
const error_messages = {
width: t('minWidthError'),
height: t('minHeightError'),
filename: t('emptyFilenameError'),
};
const renderErrorHandler = errorType => {
if (!error[errorType]) {
return null;
}
return <div className="input-error">{error_messages[errorType]}</div>;
};
const onKeepAspectToggle = () => {
const { width, height } = dimensions;
const aspectMultiplier = { ...aspectMultiplier };
if (!keepAspect) {
const base = Math.min(width, height);
aspectMultiplier.width = width / base;
aspectMultiplier.height = height / base;
setAspectMultiplier(aspectMultiplier);
}
setKeepAspect(!keepAspect);
};
const validSize = value => (value >= minimumSize ? value : minimumSize);
const loadAndUpdateViewports = useCallback(async () => {
const { width: scaledWidth, height: scaledHeight } = await loadImage(
activeViewport,
viewportElement,
dimensions.width,
dimensions.height
);
toggleAnnotations(showAnnotations, viewportElement);
const scaledDimensions = {
height: validSize(scaledHeight),
width: validSize(scaledWidth),
};
loadAndUpdateViewports();
setViewportElementDimensions(scaledDimensions);
setDownloadCanvas(state => ({
...state,
...scaledDimensions,
}));
const {
dataUrl,
width: viewportElementWidth,
height: viewportElementHeight,
} = await updateViewportPreview(
viewportElement,
downloadCanvas.ref.current,
fileType
);
setViewportPreview(state => ({
...state,
src: dataUrl,
width: validSize(viewportElementWidth),
height: validSize(viewportElementHeight),
}));
}, [
activeViewport,
viewportElement,
@ -111,76 +213,96 @@ const ViewportDownloadForm = ({
viewportElementDimensions,
]);
/**
* @param {object} event - Input change event
* @param {string} dimension - "height" | "width"
*/
const onDimensionsChange = (event, dimension) => {
const sanitizedTargetValue = event.target.value.replace(/\D/, '');
const isEmpty = sanitizedTargetValue === '';
const updatedDimension = isEmpty
? ''
: Math.min(sanitizedTargetValue, maximumSize);
useEffect(() => {
enableViewport(viewportElement);
if (updatedDimension === dimensions.width) {
return;
return () => {
disableViewport(viewportElement);
};
}, [disableViewport, enableViewport, viewportElement]);
useEffect(() => {
if (refreshViewport.current !== null) {
clearTimeout(refreshViewport.current);
}
// In current code, keepAspect is always `true`
// And we always start w/ a square width/height
setDimensions({
width: updatedDimension,
height: updatedDimension,
});
refreshViewport.current = setTimeout(() => {
refreshViewport.current = null;
loadAndUpdateViewports();
}, REFRESH_VIEWPORT_TIMEOUT);
}, [
activeViewport,
viewportElement,
showAnnotations,
dimensions,
loadImage,
toggleAnnotations,
updateViewportPreview,
fileType,
downloadCanvas.ref,
minimumSize,
maximumSize,
]);
// Only update if value is non-empty
if (!isEmpty) {
setViewportElementDimensions({
height: updatedDimension,
width: updatedDimension,
});
setDownloadCanvas(state => ({
...state,
height: updatedDimension,
width: updatedDimension,
}));
}
};
useEffect(() => {
const { width, height } = dimensions;
const hasError = {
width: width < minimumSize,
height: height < minimumSize,
filename: !filename,
};
const downloadImage = () => {
downloadBlob(
filename || DEFAULT_FILENAME,
fileType,
viewportElement,
downloadCanvas.ref.current
);
};
setError({ ...hasError });
}, [dimensions, filename, minimumSize]);
return (
<div className="ViewportDownloadForm">
<div className="title">
{t(
'Please specify the dimensions, filename, and desired type for the output image.'
)}
</div>
<div className="title">{t('formTitle')}</div>
<div className="file-info-container" data-cy="file-info-container">
<div className="col">
<div className="width">
<TextInput
data-cy="image-width"
value={dimensions.width}
label={t('Image width (px)')}
onChange={evt => onDimensionsChange(evt, 'height')}
/>
<div className="dimension-wrapper">
<div className="dimensions">
<div className="width">
<TextInput
type="number"
min={minimumSize}
max={maximumSize}
value={dimensions.width}
label={t('imageWidth')}
onChange={evt => onDimensionsChange(evt, 'width')}
data-cy="image-width"
/>
{renderErrorHandler('width')}
</div>
<div className="height">
<TextInput
type="number"
min={minimumSize}
max={maximumSize}
value={dimensions.height}
label={t('imageHeight')}
onChange={evt => onDimensionsChange(evt, 'height')}
data-cy="image-height"
/>
{renderErrorHandler('height')}
</div>
</div>
<div className="height">
<TextInput
data-cy="image-height"
value={dimensions.height}
label={t('Image height (px)')}
onChange={evt => onDimensionsChange(evt, 'width')}
/>
<div className="keep-aspect-wrapper">
<button
id="keep-aspect"
className={classnames(
'form-button btn',
keepAspect ? 'active' : ''
)}
data-cy="keep-aspect"
alt={t('keepAspectRatio')}
onClick={onKeepAspectToggle}
>
<Icon
name={keepAspect ? 'link' : 'unlink'}
alt={keepAspect ? 'Dismiss Aspect' : 'Keep Aspect'}
/>
</button>
</div>
</div>
@ -191,9 +313,10 @@ const ViewportDownloadForm = ({
data-cy="file-name"
value={filename}
onChange={event => setFilename(event.target.value)}
label={t('File name')}
label={t('filename')}
id="file-name"
/>
{renderErrorHandler('filename')}
</div>
<div className="file-type">
<Select
@ -201,7 +324,7 @@ const ViewportDownloadForm = ({
data-cy="file-type"
onChange={event => setFileType(event.target.value)}
options={FILE_TYPE_OPTIONS}
label={t('File type')}
label={t('fileType')}
/>
</div>
</div>
@ -217,7 +340,7 @@ const ViewportDownloadForm = ({
checked={showAnnotations}
onChange={event => setShowAnnotations(event.target.checked)}
/>
{t('Show Annotations')}
{t('showAnnotations')}
</label>
</div>
</div>
@ -245,15 +368,23 @@ const ViewportDownloadForm = ({
></canvas>
</div>
<div className="preview" data-cy="image-preview">
<h4> {t('Image Preview')}</h4>
<img
className="viewport-preview"
src={viewportPreview.src}
alt="Viewport Preview"
data-cy="viewport-preview-img"
/>
</div>
{viewportPreview.src ? (
<div className="preview" data-cy="image-preview">
<div className="preview-header"> {t('imagePreview')}</div>
<img
className="viewport-preview"
src={viewportPreview.src}
alt={t('imagePreview')}
data-cy="image-preview"
data-cy="viewport-preview-img"
/>
</div>
) : (
<div className="loading-image">
<Icon name="circle-notch" className="icon-spin" />
{t('loadingPreview')}
</div>
)}
<div className="actions">
<div className="action-cancel">
@ -263,16 +394,17 @@ const ViewportDownloadForm = ({
className="btn btn-danger"
onClick={onClose}
>
{t('Cancel')}
{t('Buttons:Cancel')}
</button>
</div>
<div className="action-save">
<button
disabled={hasError}
onClick={downloadImage}
className="btn btn-primary"
data-cy="download-btn"
>
{t('Download')}
{t('Buttons:Download')}
</button>
</div>
</div>

View File

@ -10,6 +10,9 @@
input, select
max-height: 30px;
#keep-aspect svg
margin-top: 3px;
.title
margin: 0;
font-weight: bold;
@ -17,85 +20,117 @@
.file-info-container
display: flex;
flex-direction: row;
justify-content: space-around;
margin: 20px 0;
border-radius: 5px;
padding: 20px 10px 0;
background-color: #16202b;
.form-control.input-ohif
padding: 6px 12px;
padding: 20px 10px;
background-color: var(--ui-gray-dark);
@media screen and (max-width: 1023px)
flex-direction: column;
align-items: flex-start;
.col
flex-grow: 1;
.width,
.height,
.file-name,
.file-type
height: 56px;
.input-ohif
margin-left: 15px;
.input-ohif
margin: 0 5px;
.file-name,
.file-type
.select-ohif, .input-ohif
width: 170px;
.input-ohif-label, .select-ohif-label
width: 90px;
display: inline-block;
@media screen and (max-width: 1023px)
margin-left: 0;
margin-top: 5px;
width: 100%;
width: 120px;
.file-type
.select-ohif
margin-left: 17px;
.dimension-wrapper
display: flex;
flex-direction: row;
.dimensions
display: flex;
flex-direction: column;
.input-ohif-label
width: 120px;
display: inline-block;
.input-ohif
@media screen and (max-width: 1023px)
margin-left: 0;
width: 100%;
width: 170px;
.show-annotations
font-weight: bold;
line-height: 30px;
input
margin-right: 7px;
vertical-align: middle;
label
display: flex;
justify-content: center;
align-items: center;
.keep-aspect-wrapper
display: flex;
justify-content: center;
align-items: center;
padding: 0 10px;
height: 86px;
.show-annotations
font-weight: bold;
line-height: 30px;
input
margin-right: 7px;
vertical-align: middle;
label
display: flex;
justify-content: center;
align-items: center;
.loading-image
height: 580px;
display: flex;
justify-content: center;
align-items: center;
color: var(--active-color);
font-size: 20px;
.icon-spin
margin-right: 15px;
.preview
display: flex;
flex-direction column;
height: fit-content;
background-color: #16202b;
width: fit-content;
flex-direction: column;
background-color: var(--ui-gray-dark);
padding: 10px;
border-radius: 5px;
align-self: center;
margin-bottom: 20px;
@media screen and (max-width: 1023px)
width: 100%;
justify-content: center;
align-items: center;
justify-content: flex-start;
align-items: center;
height: 580px;
.viewport-preview
max-height: 512px;
max-width: 512px;
h4
.preview-header
width: 100%;
text-align center;
font-size: 1.3em;
margin: 0 0 10px;
.preview-container
width: auto;
height: 100%;
max-height: 400px;
object-fit contain;
.actions
display: flex;
flex-wrap: nowrap;
justify-content: flex-end;
align-items: center;
.action-cancel
margin: 0 20px;
.actions-save
margin: 0 0 0 10px;
margin-top: 20px;
.btn
margin: 0 10px;
.input-error
font-size: 12px;
color: red;
text-align: center;
margin: 3px 0;
.modal-dialog
height: 100%;

View File

@ -10,7 +10,10 @@ export class TableListItem extends Component {
children: PropTypes.node,
itemClass: PropTypes.string,
itemIndex: PropTypes.number,
itemKey: PropTypes.oneOfType(['number', 'string']),
itemKey: PropTypes.oneOfType([
PropTypes.string,
PropTypes.number,
]),
onItemClick: PropTypes.func.isRequired,
};

View File

@ -1,6 +1,10 @@
.icon-pulse
fa-spin 1s infinite steps(8)
.icon-spin {
animation: spin 2s linear infinite;
}
@keyframes fa-spin{
0%{ transform:rotate(0deg) }
to{ transform:rotate(1turn) }

View File

@ -80,6 +80,7 @@ import thLarge from './icons/th-large.svg';
import thList from './icons/th-list.svg';
import times from './icons/times.svg';
import trash from './icons/trash.svg';
import unlink from './icons/unlink.svg';
import user from './icons/user.svg';
import youtube from './icons/youtube.svg';
@ -158,6 +159,7 @@ const ICONS = {
rotate,
'rotate-right': rotateRight,
trash,
unlink,
'exclamation-circle': exclamationCircle,
link,
'exclamation-triangle': exclamationTriangle,

View File

@ -0,0 +1,11 @@
<svg
xmlns="http://www.w3.org/2000/svg"
aria-labelledby="unlink"
viewBox="0 0 512 512"
width="1em"
height="1em"
fill="currentColor"
>
<title id="title">Unlink</title>
<path d="M304.083 388.936c4.686 4.686 4.686 12.284 0 16.971l-65.057 65.056c-54.709 54.711-143.27 54.721-197.989 0-54.713-54.713-54.719-143.27 0-197.989l65.056-65.057c4.686-4.686 12.284-4.686 16.971 0l22.627 22.627c4.686 4.686 4.686 12.284 0 16.971L81.386 311.82c-34.341 34.341-33.451 88.269.597 120.866 32.577 31.187 84.788 31.337 117.445-1.32l65.057-65.056c4.686-4.686 12.284-4.686 16.971 0l22.627 22.626zm-56.568-243.245l64.304-64.304c34.346-34.346 88.286-33.453 120.882.612 31.18 32.586 31.309 84.785-1.335 117.43l-65.056 65.057c-4.686 4.686-4.686 12.284 0 16.971l22.627 22.627c4.686 4.686 12.284 4.686 16.971 0l65.056-65.057c54.711-54.709 54.721-143.271 0-197.99-54.71-54.711-143.27-54.72-197.989 0l-65.057 65.057c-4.686 4.686-4.686 12.284 0 16.971l22.627 22.627c4.685 4.685 12.283 4.685 16.97-.001zm238.343 362.794l22.627-22.627c4.686-4.686 4.686-12.284 0-16.971L43.112 3.515c-4.686-4.686-12.284-4.686-16.971 0L3.515 26.142c-4.686 4.686-4.686 12.284 0 16.971l465.373 465.373c4.686 4.686 12.284 4.686 16.97-.001z"></path>
</svg>

After

Width:  |  Height:  |  Size: 1.2 KiB

View File

@ -29,18 +29,18 @@ class Select extends Component {
render() {
return (
<div className="select-ohif-container">
<label className="select-ohif-label" htmlFor={this.id}>
{this.props.label}
<select className="form-control select-ohif" {...this.props}>
{this.props.options.map(({ key, value }) => {
return (
<option key={key} value={value}>
{key}
</option>
);
})}
</select>
</label>
{this.props.label && (
<label className="select-ohif-label" htmlFor={this.id}>{this.props.label}</label>
)}
<select className="form-control select-ohif" {...this.props}>
{this.props.options.map(({ key, value }) => {
return (
<option key={key} value={value}>
{key}
</option>
);
})}
</select>
</div>
);
}

View File

@ -14,7 +14,7 @@ class TextInput extends React.Component {
PropTypes.number
]),
id: PropTypes.string,
label:PropTypes.string,
label: PropTypes.string,
type: PropTypes.string,
};
@ -28,15 +28,15 @@ class TextInput extends React.Component {
render() {
return (
<div className="input-ohif-container">
<label className="input-ohif-label" htmlFor={this.props.id}>
{this.props.label}
<input
type={this.props.type}
id={this.props.id}
className="form-control input-ohif"
{...this.props}
/>
</label>
{this.props.label && (
<label className="input-ohif-label" htmlFor={this.props.id}>{this.props.label}</label>
)}
<input
type={this.props.type}
id={this.props.id}
className="form-control input-ohif"
{...this.props}
/>
</div>
);
}

View File

@ -16,14 +16,14 @@ describe('OHIF Download Snapshot File', () => {
.click();
});
it('checks displayed information for Tablet experience', function() {
it('checks displayed information for Tablet experience', function () {
// Set Tablet resolution
cy.viewport(1000, 660);
// Visual comparison
cy.screenshot('Download Image Modal - Tablet experience');
});
it('checks displayed information for Desktop experience', function() {
it('checks displayed information for Desktop experience', function () {
// Set Desktop resolution
cy.viewport(1750, 720);
// Visual comparison
@ -52,7 +52,7 @@ describe('OHIF Download Snapshot File', () => {
.should('be.visible');
});
it('cancel changes on download modal', function() {
it('cancel changes on download modal', function () {
//Change Image Width, Filename and File Type
cy.get('[data-cy="image-width"]')
.clear()
@ -91,7 +91,7 @@ describe('OHIF Download Snapshot File', () => {
// //Check error message
// });
it('checks if "Show Annotations" checkbox will display annotations', function() {
it('checks if "Show Annotations" checkbox will display annotations', function () {
// Close modal that is initially opened
cy.get('[data-cy="close-button"]').click();