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

Ensure unique volume layer names when changing names in frontend #6289

Merged
merged 11 commits into from
Jul 4, 2022
Merged
1 change: 1 addition & 0 deletions CHANGELOG.unreleased.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ For upgrade instructions, please check the [migration guide](MIGRATIONS.released

### Added
- Added a image data download speed indicator to the statusbar. On hover a tooltip is shown that show the total amount of downloaded shard data. [#6269](https://github.com/scalableminds/webknossos/pull/6269)
- Added a warning for invalid volume layer names. The layer names must now be unique among all layers in an annotation and must not contain a `/`. [#6289](https://github.com/scalableminds/webknossos/pull/6289)
- Added a warning for when the resolution in the XY viewport on z=1-downsampled datasets becomes too low, explaining the problem and how to mitigate it. [#6255](https://github.com/scalableminds/webknossos/pull/6255)
- Added the possibility to view and download older versions of read-only annotations. [#6274](https://github.com/scalableminds/webknossos/pull/6274)

Expand Down
10 changes: 6 additions & 4 deletions frontend/javascripts/admin/user/user_list_view.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -444,10 +444,12 @@ class UserListView extends React.PureComponent<Props, State> {
<EditableTextLabel
value={user.email}
label="Email"
rules={{
message: messages["auth.registration_email_invalid"],
type: "email",
}}
rules={[
{
message: messages["auth.registration_email_invalid"],
type: "email",
},
]}
onChange={(newEmail) => {
if (newEmail !== user.email) {
Modal.confirm({
Expand Down
4 changes: 4 additions & 0 deletions frontend/javascripts/messages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,10 @@ instead. Only enable this option if you understand its effect. All layers will n
"tracing.read_only_mode_notification":
"This annotation is in read-only mode and cannot be updated.",
"tracing.volume_missing_segmentation": "Volume is allowed, but segmentation does not exist.",
"tracing.volume_layer_name_duplication":
"This layer name already exists! Please change it to resolve duplicates.",
"tracing.volume_layer_name_includes_slash":
'This layer name includes the disallowed character "/". Please delete it.',
"tracing.delete_initial_node": "Do you really want to delete the initial node?",
"tracing.delete_tree": "Do you really want to delete the whole tree?",
"tracing.delete_tree_with_initial_node":
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,10 @@ import {
getSegmentationLayerByName,
getSegmentationLayers,
getVisibleSegmentationLayer,
getDataLayers,
} from "oxalis/model/accessors/dataset_accessor";
import _ from "lodash";
import messages from "messages";
import { getMaxZoomStepDiff } from "oxalis/model/bucket_data_handling/loading_strategy_logic";
import { getFlooredPosition, getRequestLogZoomStep } from "oxalis/model/accessors/flycam_accessor";
import { reuseInstanceOnEquality } from "oxalis/model/accessors/accessor_helpers";
Expand Down Expand Up @@ -94,6 +97,56 @@ export function getReadableNameByVolumeTracingId(
return volumeDescriptor.name || "Volume Layer";
}

export function getAllReadableLayerNames(dataset: APIDataset, tracing: Tracing) {
const allReadableLayerNames = getDataLayers(dataset).map((currentLayer) =>
"tracingId" in currentLayer && currentLayer.tracingId != null
? getReadableNameByVolumeTracingId(tracing, currentLayer.tracingId)
: currentLayer.name,
);
if (tracing.skeleton != null) {
allReadableLayerNames.push("Skeletons");
}
return allReadableLayerNames;
}

export function checkForLayerNameDuplication(
readableLayerName: string,
dataset: APIDataset,
tracing: Tracing,
currentName?: string | null,
): string | true {
let countToFindDuplication = 1;
if (readableLayerName === currentName) {
countToFindDuplication = 2;
}
const allReadableLayerNames = getAllReadableLayerNames(dataset, tracing);
daniel-wer marked this conversation as resolved.
Show resolved Hide resolved
const doesNewNameAlreadyExist =
_.countBy(allReadableLayerNames)[readableLayerName] >= countToFindDuplication;
return doesNewNameAlreadyExist ? messages["tracing.volume_layer_name_duplication"] : true;
}

export function checkForInvalidCharacters(readableLayerName: string): string | true {
MichaelBuessemeyer marked this conversation as resolved.
Show resolved Hide resolved
return readableLayerName.includes("/")
? messages["tracing.volume_layer_name_includes_slash"]
: true;
}

export function validateReadableLayerName(
readableLayerName: string,
dataset: APIDataset,
tracing: Tracing,
dontCountGivenName?: boolean,
): string | true {
MichaelBuessemeyer marked this conversation as resolved.
Show resolved Hide resolved
const hasDuplicatedName = checkForLayerNameDuplication(
readableLayerName,
dataset,
tracing,
dontCountGivenName ? readableLayerName : null,
);
const nameContainsSlash = checkForInvalidCharacters(readableLayerName);
return hasDuplicatedName === true ? nameContainsSlash : hasDuplicatedName;
}

MichaelBuessemeyer marked this conversation as resolved.
Show resolved Hide resolved
function getSegmentationLayerForTracing(
state: OxalisState,
volumeTracing: VolumeTracing,
Expand Down
61 changes: 46 additions & 15 deletions frontend/javascripts/oxalis/view/components/editable_text_label.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,12 @@ import Toast from "libs/toast";
type Rule = {
message?: string;
type?: string;
validator?: (arg0: number | string) => boolean;
MichaelBuessemeyer marked this conversation as resolved.
Show resolved Hide resolved
};
export type EditableTextLabelProp = {
value: string;
onChange: (...args: Array<any>) => any;
rules?: Rule;
rules?: Rule[];
rows?: number;
markdown?: boolean;
label: string;
Expand All @@ -21,6 +22,9 @@ export type EditableTextLabelProp = {
disableEditing?: boolean;
onContextMenu?: () => void;
width?: string | number;
updateOnAllChanges?: boolean;
MichaelBuessemeyer marked this conversation as resolved.
Show resolved Hide resolved
isInvalid?: boolean | null | undefined;
trimValue?: boolean | null | undefined;
};
type State = {
isEditing: boolean;
Expand All @@ -30,7 +34,12 @@ type State = {
class EditableTextLabel extends React.PureComponent<EditableTextLabelProp, State> {
static defaultProps = {
rows: 1,
updateOnAllChanges: false,
isInvalid: false,
trimValue: false,
rules: [],
};

state: State = {
isEditing: false,
value: "",
Expand All @@ -56,26 +65,46 @@ class EditableTextLabel extends React.PureComponent<EditableTextLabelProp, State
value: event.target.value,
});
};

handleOnChange = () => {
if (this.validateFields()) {
this.props.onChange(this.state.value);
this.setState({
isEditing: false,
});
const validateAndUpdateValue = () => {
if (this.validateFields()) {
this.props.onChange(this.state.value);
this.setState({
isEditing: false,
});
}
};
if (this.props.trimValue) {
this.setState(
(prevState) => ({ value: prevState.value.trim() }),
// afterwards validate
validateAndUpdateValue,
);
} else {
validateAndUpdateValue();
Comment on lines +69 to +84
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here I avoid having a volume layer name with trailing white spaces. Previous Layer1 and Layer1 was possible and not counted as a name duplication. This is fixed by setting the prop trimValue in the EditableTextLabel.

}
};

validateFields() {
if (this.props.rules && this.props.rules.type === "email") {
const re =
/^(([^<>()[\]\\.,;:\s@"]+(\.[^<>()[\]\\.,;:\s@"]+)*)|(".+"))@((\[[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\])|(([a-zA-Z\-0-9]+\.)+[a-zA-Z]{2,}))$/;
const matchesEmail = re.test(this.state.value);
if (!matchesEmail && this.props.rules && this.props.rules.message)
Toast.error(this.props.rules.message);
return matchesEmail;
} else {
if (!this.props.rules) {
return true;
}
const allRulesValid = this.props.rules.every((rule) => {
let isValid = true;
if (rule.type === "email") {
const re =
/^(([^<>()[\]\\.,;:\s@"]+(\.[^<>()[\]\\.,;:\s@"]+)*)|(".+"))@((\[[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\])|(([a-zA-Z\-0-9]+\.)+[a-zA-Z]{2,}))$/;
isValid = re.test(this.state.value);
} else if (rule.validator != null) {
isValid = rule.validator(this.state.value);
}
if (!isValid && rule.message) {
Toast.error(rule.message);
}
return isValid;
});
return allRulesValid;
}

render() {
Expand All @@ -96,6 +125,7 @@ class EditableTextLabel extends React.PureComponent<EditableTextLabelProp, State
// @ts-ignore
rows: this.props.rows,
};
const isInvalidStyleMaybe = this.props.isInvalid ? { color: "var(--ant-error)" } : {};

if (this.state.isEditing) {
return (
Expand Down Expand Up @@ -148,9 +178,10 @@ class EditableTextLabel extends React.PureComponent<EditableTextLabelProp, State
linkify: true,
}}
container="span"
style={isInvalidStyleMaybe}
/>
) : (
this.props.value
<span style={isInvalidStyleMaybe}>{this.props.value}</span>
)}
{this.props.disableEditing ? null : (
<Tooltip key="edit" title={`Edit ${this.props.label}`} placement="bottom">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,12 @@ import {
} from "oxalis/model/accessors/dataset_accessor";
import { getMaxZoomValueForResolution } from "oxalis/model/accessors/flycam_accessor";
import {
checkForInvalidCharacters,
checkForLayerNameDuplication,
getReadableNameByVolumeTracingId,
getVolumeDescriptorById,
getVolumeTracingById,
validateReadableLayerName,
} from "oxalis/model/accessors/volumetracing_accessor";
import {
setNodeRadiusAction,
Expand Down Expand Up @@ -80,7 +83,7 @@ import Store from "oxalis/store";
import Toast from "libs/toast";
import * as Utils from "libs/utils";
import api from "oxalis/api/internal_api";
import { settings } from "messages";
import messages, { settings } from "messages";
import { MaterializeVolumeAnnotationModal } from "oxalis/view/right-border-tabs/starting_job_modals";
import AddVolumeLayerModal from "./modals/add_volume_layer_modal";
import DownsampleVolumeModal from "./modals/downsample_volume_modal";
Expand Down Expand Up @@ -304,9 +307,9 @@ class DatasetSettings extends React.PureComponent<DatasetSettingsProps, State> {
elementClass: string,
layerSettings: DatasetLayerConfiguration,
) => {
const { tracing } = this.props;
const { tracing, dataset } = this.props;
const { intensityRange } = layerSettings;
const layer = getLayerByName(this.props.dataset, layerName);
const layer = getLayerByName(dataset, layerName);
const isVolumeTracing = layer.category === "segmentation" ? layer.tracingId != null : false;
const maybeTracingId = layer.category === "segmentation" ? layer.tracingId : null;
const maybeVolumeTracing =
Expand Down Expand Up @@ -335,7 +338,6 @@ class DatasetSettings extends React.PureComponent<DatasetSettingsProps, State> {
setSingleLayerVisibility(true);
}
};

const hasHistogram = this.props.histogramData[layerName] != null;
const resolutions = getResolutionInfo(layer.resolutions).getResolutionList();
const volumeDescriptor =
Expand All @@ -346,6 +348,12 @@ class DatasetSettings extends React.PureComponent<DatasetSettingsProps, State> {
"tracingId" in layer && layer.tracingId != null
? getReadableNameByVolumeTracingId(tracing, layer.tracingId)
: layerName;
const isReadableNameValidOrWarning = validateReadableLayerName(
readableName,
dataset,
tracing,
true,
);
return (
<div className="flex-container">
{this.getEnableDisableLayerSwitch(isDisabled, onChange)}
Expand All @@ -357,17 +365,46 @@ class DatasetSettings extends React.PureComponent<DatasetSettingsProps, State> {
}}
>
{volumeDescriptor != null ? (
<EditableTextLabel
margin="0 10px 0 0"
width={150}
value={readableName}
onChange={(newName) => {
this.props.onEditAnnotationLayer(volumeDescriptor.tracingId, {
name: newName,
});
}}
label="Volume Layer Name"
/>
<Tooltip
title={isReadableNameValidOrWarning === true ? null : isReadableNameValidOrWarning}
>
<span style={{ display: "inline-block" }}>
<EditableTextLabel
margin="0 10px 0 0"
width={150}
value={readableName}
isInvalid={isReadableNameValidOrWarning !== true}
trimValue
onChange={(newName) => {
this.props.onEditAnnotationLayer(volumeDescriptor.tracingId, {
name: newName,
});
}}
rules={[
{
message: messages["tracing.volume_layer_name_duplication"],
validator: (newLayerName) =>
typeof newLayerName === "string"
daniel-wer marked this conversation as resolved.
Show resolved Hide resolved
? checkForLayerNameDuplication(
newLayerName,
dataset,
tracing,
readableName,
) === true
: false,
},
{
message: messages["tracing.volume_layer_name_includes_slash"],
validator: (newLayerName) =>
daniel-wer marked this conversation as resolved.
Show resolved Hide resolved
typeof newLayerName === "string"
? checkForInvalidCharacters(newLayerName) === true
: false,
},
]}
label="Volume Layer Name"
/>
</span>
</Tooltip>
) : (
layerName
)}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { Modal, Row } from "antd";
import { PlusOutlined } from "@ant-design/icons";
import React, { useState } from "react";
import React, { useMemo, useState } from "react";
import _ from "lodash";
import type { APIDataset } from "types/api_flow_types";
import { AsyncButton } from "components/async_clickables";
Expand All @@ -14,9 +14,14 @@ import {
getDatasetResolutionInfo,
getSegmentationLayers,
} from "oxalis/model/accessors/dataset_accessor";
import { getVolumeTracingLayers } from "oxalis/model/accessors/volumetracing_accessor";
import {
getAllReadableLayerNames,
getVolumeTracingLayers,
validateReadableLayerName,
} from "oxalis/model/accessors/volumetracing_accessor";
import InputComponent from "oxalis/view/components/input_component";
import api from "oxalis/api/internal_api";
import Toast from "libs/toast";
export default function AddVolumeLayerModal({
dataset,
onCancel,
Expand All @@ -29,7 +34,22 @@ export default function AddVolumeLayerModal({
const [selectedSegmentationLayerIndex, setSelectedSegmentationLayerIndex] = useState<
number | null | undefined
>(null);
const [newLayerName, setNewLayerName] = useState("Volume");
const initialNewLayerName = useMemo(() => {
const allReadableLayerNames = getAllReadableLayerNames(dataset, tracing);
if (allReadableLayerNames.indexOf("Volume") === -1) {
return "Volume";
} else {
let counter = 1;
let name = `Volume${counter}`;
while (allReadableLayerNames.indexOf(name) >= 0) {
// Increase number at the end of name until a valid initial name is found.
++counter;
name = `Volume${counter}`;
}
return name;
}
}, [dataset, tracing]);
const [newLayerName, setNewLayerName] = useState(initialNewLayerName);

const datasetResolutionInfo = getDatasetResolutionInfo(dataset);
const [resolutionIndices, setResolutionIndices] = useState([0, 10000]);
Expand All @@ -44,7 +64,11 @@ export default function AddVolumeLayerModal({
let selectedSegmentationLayer = null;
const handleAddVolumeLayer = async () => {
await api.tracing.save();

const isNewLayerNameValidOrWarning = validateReadableLayerName(newLayerName, dataset, tracing);
if (isNewLayerNameValidOrWarning !== true) {
Toast.error(isNewLayerNameValidOrWarning);
return;
}
const minResolutionAllowed = Math.max(
...datasetResolutionInfo.getResolutionByIndexOrThrow(resolutionIndices[0]),
);
Expand Down