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 @@ -11,6 +11,7 @@ For upgrade instructions, please check the [migration guide](MIGRATIONS.released
[Commits](https://github.com/scalableminds/webknossos/compare/22.07.0...HEAD)

### Added
- 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 url encoded special characters. [#6289](https://github.com/scalableminds/webknossos/pull/6289)
- Added optional mappingName parameter to `requestRawCuboid` datastore route, which allows to directly apply a specified mapping in the backend. [#6311](https://github.com/scalableminds/webknossos/pull/6311)
- Added option to use `X-Auth-Token` header instead of query parameter in every datastore and tracingstore route. [#6312](https://github.com/scalableminds/webknossos/pull/6312)

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
6 changes: 6 additions & 0 deletions frontend/javascripts/messages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,12 @@ 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_invalid_characters": (disallowedCharacters: string) =>
`This layer name includes the disallowed character${
disallowedCharacters.length > 1 ? "s" : ""
} "${disallowedCharacters}". Please delete ${disallowedCharacters.length > 1 ? "them" : "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,6 +26,7 @@ import {
getSegmentationLayerByName,
getSegmentationLayers,
getVisibleSegmentationLayer,
getDataLayers,
} from "oxalis/model/accessors/dataset_accessor";
import { getMaxZoomStepDiff } from "oxalis/model/bucket_data_handling/loading_strategy_logic";
import { getFlooredPosition, getRequestLogZoomStep } from "oxalis/model/accessors/flycam_accessor";
Expand Down Expand Up @@ -94,6 +95,18 @@ 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;
}

function getSegmentationLayerForTracing(
state: OxalisState,
volumeTracing: VolumeTracing,
Expand Down
64 changes: 49 additions & 15 deletions frontend/javascripts/oxalis/view/components/editable_text_label.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,16 @@ import Markdown from "react-remarkable";
import * as React from "react";
import { MarkdownModal } from "oxalis/view/components/markdown_modal";
import Toast from "libs/toast";
import { ValidationResult } from "../left-border-tabs/modals/add_volume_layer_modal";
type Rule = {
message?: string;
type?: string;
validator?: (arg0: string) => ValidationResult;
};
export type EditableTextLabelProp = {
value: string;
onChange: (...args: Array<any>) => any;
rules?: Rule;
rules?: Rule[];
rows?: number;
markdown?: boolean;
label: string;
Expand All @@ -21,6 +23,8 @@ export type EditableTextLabelProp = {
disableEditing?: boolean;
onContextMenu?: () => void;
width?: string | number;
isInvalid?: boolean | null | undefined;
trimValue?: boolean | null | undefined;
};
type State = {
isEditing: boolean;
Expand All @@ -30,7 +34,11 @@ type State = {
class EditableTextLabel extends React.PureComponent<EditableTextLabelProp, State> {
static defaultProps = {
rows: 1,
isInvalid: false,
trimValue: false,
rules: [],
};

state: State = {
isEditing: false,
value: "",
Expand All @@ -56,26 +64,50 @@ 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) => {
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,}))$/;
const isValid = re.test(this.state.value);
if (!isValid) {
Toast.error(rule.message);
return false;
}
} else if (rule.validator != null) {
const validationResult = rule.validator(this.state.value);
if (!validationResult.isValid) {
Toast.error(validationResult.message);
return false;
}
}
return true;
});
return allRulesValid;
}

render() {
Expand All @@ -96,6 +128,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 +181,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,6 +45,7 @@ import {
} from "oxalis/model/accessors/dataset_accessor";
import { getMaxZoomValueForResolution } from "oxalis/model/accessors/flycam_accessor";
import {
getAllReadableLayerNames,
getReadableNameByVolumeTracingId,
getVolumeDescriptorById,
getVolumeTracingById,
Expand Down Expand Up @@ -82,7 +83,7 @@ import * as Utils from "libs/utils";
import api from "oxalis/api/internal_api";
import { settings } from "messages";
import { MaterializeVolumeAnnotationModal } from "oxalis/view/right-border-tabs/starting_job_modals";
import AddVolumeLayerModal from "./modals/add_volume_layer_modal";
import AddVolumeLayerModal, { validateReadableLayerName } from "./modals/add_volume_layer_modal";
import DownsampleVolumeModal from "./modals/downsample_volume_modal";
import Histogram, { isHistogramSupported } from "./histogram_view";
import MappingSettingsView from "./mapping_settings_view";
Expand Down Expand Up @@ -304,9 +305,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 +336,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 +346,12 @@ class DatasetSettings extends React.PureComponent<DatasetSettingsProps, State> {
"tracingId" in layer && layer.tracingId != null
? getReadableNameByVolumeTracingId(tracing, layer.tracingId)
: layerName;
const allReadableLayerNames = getAllReadableLayerNames(dataset, tracing);
const readableLayerNameValidationResult = validateReadableLayerName(
readableName,
allReadableLayerNames,
readableName,
);
return (
<div className="flex-container">
{this.getEnableDisableLayerSwitch(isDisabled, onChange)}
Expand All @@ -357,17 +363,39 @@ 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={
readableLayerNameValidationResult.isValid
? null
: readableLayerNameValidationResult.message
}
>
<span style={{ display: "inline-block" }}>
<EditableTextLabel
margin="0 10px 0 0"
width={150}
value={readableName}
isInvalid={!readableLayerNameValidationResult.isValid}
trimValue
onChange={(newName) => {
this.props.onEditAnnotationLayer(volumeDescriptor.tracingId, {
name: newName,
});
}}
rules={[
{
validator: (newReadableLayerName) =>
validateReadableLayerName(
newReadableLayerName,
allReadableLayerNames,
readableName,
),
},
]}
label="Volume Layer Name"
/>
</span>
</Tooltip>
) : (
layerName
)}
Expand Down
Loading