Skip to content

fix: BROS-136: Don't allow to create regions in View All #7907

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

Merged
merged 7 commits into from
Jul 7, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@
} = this.props;

const activeStates = parent?.activeStates();
const statesSelected = activeStates && activeStates.length;
const statesSelected = activeStates?.length;

Check warning on line 100 in web/libs/editor/src/components/TimeSeries/TimeSeriesVisualizer.jsx

View check run for this annotation

Codecov / codecov/patch

web/libs/editor/src/components/TimeSeries/TimeSeriesVisualizer.jsx#L100

Added line #L100 was not covered by tests
const readonly = parent?.annotation?.isReadOnly();

// skip if event fired by .move() - prevent recursion and bugs
Expand All @@ -108,10 +108,13 @@
const x = d3.mouse(d3.event.sourceEvent.target)[0];
const newRegion = this.newRegion;

// double click handler to create instant region
// when 2nd click happens during 300ms after 1st click and in the same place
if (newRegion && Math.abs(newRegion.x - x) < 4) {
clearTimeout(this.newRegionTimer);
parent?.regionChanged(newRegion.range, ranges.length, newRegion.states);
if (!readonly) {
parent?.regionChanged(newRegion.range, ranges.length, newRegion.states);

Check warning on line 116 in web/libs/editor/src/components/TimeSeries/TimeSeriesVisualizer.jsx

View check run for this annotation

Codecov / codecov/patch

web/libs/editor/src/components/TimeSeries/TimeSeriesVisualizer.jsx#L115-L116

Added lines #L115 - L116 were not covered by tests
}
this.newRegion = null;
this.newRegionTimer = null;
} else if (statesSelected) {
Expand Down
4 changes: 3 additions & 1 deletion web/libs/editor/src/components/Timeline/Timeline.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
speed,
className,
formatPosition,
readonly = false,

Check warning on line 35 in web/libs/editor/src/components/Timeline/Timeline.tsx

View check run for this annotation

Codecov / codecov/patch

web/libs/editor/src/components/Timeline/Timeline.tsx#L35

Added line #L35 was not covered by tests
...props
}) => {
const View = Views[mode];
Expand Down Expand Up @@ -106,8 +107,9 @@
seekOffset,
settings: View.settings,
visibleWidth: seekVisibleWidth,
readonly,
}),
[position, seekOffset, seekVisibleWidth, length, regions, step, playing, View.settings, data],
[position, seekOffset, seekVisibleWidth, length, regions, step, playing, View.settings, data, readonly],
);

useEffect(() => {
Expand Down
2 changes: 2 additions & 0 deletions web/libs/editor/src/components/Timeline/Types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ export interface TimelineProps<D extends ViewTypes = "frames"> {
controlsOnTop?: boolean;
controls?: TimelineControls;
customControls?: TimelineCustomControls[];
readonly?: boolean;
onReady?: (data: Record<string, any>) => void;
onPlay?: () => void;
onPause?: () => void;
Expand Down Expand Up @@ -121,6 +122,7 @@ export interface TimelineContextValue {
settings?: TimelineSettings;
changeSetting?: (key: string, value: any) => void;
data?: any;
readonly?: boolean;
}

export interface TimelineMinimapProps {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ type DataType = {
};

export const Controls: FC<TimelineExtraControls<Actions, DataType>> = ({ onAction }) => {
const { position, regions } = useContext(TimelineContext);
const { position, regions, readonly } = useContext(TimelineContext);
const hasSelectedRegion = regions.some(({ selected, timeline }) => selected && !timeline);
const closestKeypoint = useMemo(() => {
const region = regions.find((r) => r.selected && !r.timeline);
Expand Down Expand Up @@ -69,11 +69,11 @@ export const Controls: FC<TimelineExtraControls<Actions, DataType>> = ({ onActio

return (
<>
<ControlButton onClick={onKeypointToggle} disabled={!hasSelectedRegion} tooltip="Toggle Keypoint">
<ControlButton onClick={onKeypointToggle} disabled={!hasSelectedRegion || readonly} tooltip="Toggle Keypoint">
{keypointIcon}
</ControlButton>

<ControlButton onClick={onLifespanToggle} disabled={!closestKeypoint} tooltip="Toggle Interpolation">
<ControlButton onClick={onLifespanToggle} disabled={!closestKeypoint || readonly} tooltip="Toggle Interpolation">
{interpolationIcon}
</ControlButton>
</>
Expand Down
55 changes: 14 additions & 41 deletions web/libs/editor/src/tags/object/Audio/model.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,11 @@
import { getEnv, getRoot, getType, types } from "mobx-state-tree";
import { createRef } from "react";
import { customTypes } from "../../../core/CustomTypes";
import { guidGenerator } from "../../../core/Helpers.ts";
import { AnnotationMixin } from "../../../mixins/AnnotationMixin";
import IsReadyMixin from "../../../mixins/IsReadyMixin";
import ProcessAttrsMixin from "../../../mixins/ProcessAttrs";
import { SyncableMixin } from "../../../mixins/Syncable";
import { AudioRegionModel } from "../../../regions/AudioRegion";
import Utils from "../../../utils";
import { FF_LSDV_E_278, isFF } from "../../../utils/feature-flags";
import { isDefined } from "../../../utils/utilities";
import ObjectBase from "../Base";
Expand Down Expand Up @@ -156,13 +154,13 @@
activeStates() {
const states = self.states();

return states && states.filter((s) => getType(s).name === "LabelsModel" && s.isSelected);
return states?.filter((s) => getType(s).name === "LabelsModel" && s.isSelected);
},

get activeState() {
const states = self.states();

return states && states.filter((s) => getType(s).name === "LabelsModel" && s.isSelected)[0];
return states?.filter((s) => getType(s).name === "LabelsModel" && s.isSelected)[0];
},

get activeLabel() {
Expand All @@ -176,6 +174,9 @@
// use label to generate a unique key to ensure that adding/deleting can trigger changes
return labels ? labels.join(",") : "";
},
get readonly() {
return self.annotation.isReadOnly();
},
}))
////// Sync actions
.actions((self) => ({
Expand Down Expand Up @@ -219,9 +220,9 @@
////// Incoming

registerSyncHandlers() {
["play", "pause", "seek"].forEach((event) => {
for (const event of ["play", "pause", "seek"]) {
self.syncHandlers.set(event, self.handleSync);
});
}
self.syncHandlers.set("speed", self.handleSyncSpeed);
},

Expand Down Expand Up @@ -288,13 +289,13 @@
const selectedColor = activeState?.selectedColor;
const labels = activeState?.selectedValues();

selectedRegions.forEach((r) => {
for (const r of selectedRegions) {
r.update({ color: selectedColor, labels: labels ?? [] });

const region = r.isRegion ? self.updateRegion(r) : self.addRegion(r);

self.annotation.selectArea(region);
});
}

if (selectedRegions.length) {
self.requestWSUpdate();
Expand Down Expand Up @@ -341,7 +342,7 @@
(target) => target.type === "paragraphs" && target.contextscroll,
);

syncedParagraphs.forEach((paragraph) => {
for (const paragraph of syncedParagraphs) {
const segments = Object.values(paragraph.regionsStartEnd).map(({ start, end }) => ({
start,
end,
Expand All @@ -351,7 +352,7 @@
}));

self._ws.addRegions(segments);
});
}
},

handleNewRegions() {
Expand Down Expand Up @@ -381,7 +382,7 @@
},

onHotKey(e) {
e && e.preventDefault();
e?.preventDefault();

Check warning on line 385 in web/libs/editor/src/tags/object/Audio/model.js

View check run for this annotation

Codecov / codecov/patch

web/libs/editor/src/tags/object/Audio/model.js#L385

Added line #L385 was not covered by tests
self._ws.togglePlay();
return false;
},
Expand All @@ -394,34 +395,6 @@
self.playBackRate = val;
},

createRegion(wsRegion, states) {
let bgColor = self.selectedregionbg;
const st = states.find((s) => s.type === "labels");

if (st) bgColor = Utils.Colors.convertToRGBA(st.getSelectedColor(), 0.3);

const r = AudioRegionModel.create({
id: wsRegion.id ? wsRegion.id : guidGenerator(),
pid: wsRegion.pid ? wsRegion.pid : guidGenerator(),
parentID: wsRegion.parent_id === null ? "" : wsRegion.parent_id,
start: wsRegion.start,
end: wsRegion.end,
score: wsRegion.score,
readonly: wsRegion.readonly,
regionbg: self.regionbg,
selectedregionbg: bgColor,
normalization: wsRegion.normalization,
states,
});

r.setWSRegion(wsRegion);

self.regions.push(r);
self.annotation.addRegion(r);

return r;
},

addRegion(wsRegion) {
// area id is assigned to WS region during deserealization
const find_r = self.annotation.areas.get(wsRegion.id);
Expand Down Expand Up @@ -485,9 +458,9 @@
},

clearRegionMappings() {
self.regs.forEach((r) => {
for (const r of self.regs) {
r.setWSRegion(null);
});
}
},

onLoad(ws) {
Expand Down
2 changes: 0 additions & 2 deletions web/libs/editor/src/tags/object/Audio/view.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -71,8 +71,6 @@ const AudioView: FC<AudioProps> = ({ item, children, settings = {}, changeSettin
onError: item.onError,
regions: {
createable: !item.readonly,
updateable: !item.readonly,
deleteable: !item.readonly,
},
timeline: {
backgroundColor: isDarkMode ? "rgb(38, 37, 34)" : "rgba(255,255,255,0.8)",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -303,6 +303,10 @@
if (!states || states.length === 0 || ev.ctrlKey || ev.metaKey)
return this._selectRegions(ev.ctrlKey || ev.metaKey);

if (item.annotation.isReadOnly()) {
return;

Check warning on line 307 in web/libs/editor/src/tags/object/Paragraphs/HtxParagraphs.jsx

View check run for this annotation

Codecov / codecov/patch

web/libs/editor/src/tags/object/Paragraphs/HtxParagraphs.jsx#L307

Added line #L307 was not covered by tests
}

const selectedRanges = this.captureDocumentSelection();

if (selectedRanges.length === 0) {
Expand Down
13 changes: 9 additions & 4 deletions web/libs/editor/src/tags/object/TimeSeries/Channel.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@
} = this.props;

const activeStates = parent?.activeStates();
const statesSelected = activeStates && activeStates.length;
const statesSelected = activeStates?.length;
const readonly = parent?.annotation?.isReadOnly();

// skip if event fired by .move() - prevent recursion and bugs
Expand All @@ -210,10 +210,13 @@
const x = d3.mouse(d3.event.sourceEvent.target)[0];
const newRegion = this.newRegion;

// double click handler to create instant region
// when 2nd click happens during 300ms after 1st click and in the same place
if (newRegion && Math.abs(newRegion.x - x) < 4) {
clearTimeout(this.newRegionTimer);
parent?.regionChanged(newRegion.range, ranges.length, newRegion.states);
if (!readonly) {
parent?.regionChanged(newRegion.range, ranges.length, newRegion.states);

Check warning on line 218 in web/libs/editor/src/tags/object/TimeSeries/Channel.jsx

View check run for this annotation

Codecov / codecov/patch

web/libs/editor/src/tags/object/TimeSeries/Channel.jsx#L217-L218

Added lines #L217 - L218 were not covered by tests
}
this.newRegion = null;
this.newRegionTimer = null;
} else if (statesSelected) {
Expand Down Expand Up @@ -364,7 +367,7 @@
const block = this.gCreator;
const getRegion = this.getRegion;
const x = this.x;
const brush = (this.brushCreator = d3
const brush = d3
.brushX()
.extent([
[0, 0],
Expand All @@ -381,7 +384,9 @@
// replacing default filter to allow ctrl-click action
.filter(() => {
return !d3.event.button;
}));
});

this.brushCreator = brush;

this.gCreator.call(this.brushCreator);
}
Expand Down
1 change: 1 addition & 0 deletions web/libs/editor/src/tags/object/Video/HtxVideo.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -531,6 +531,7 @@ const HtxVideoView = ({ item, store }) => {
disableView={!supportsTimelineRegions && !supportsRegions}
framerate={item.framerate}
controls={{ FramesControl: true }}
readonly={item.annotation?.isReadOnly()}
customControls={[
{
position: "left",
Expand Down
13 changes: 8 additions & 5 deletions web/libs/editor/src/tags/object/Video/Video.js
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ const Model = types
// normalize framerate — should be string with number of frames per second
const framerate = Number(parseValue(self.framerate, self.store.task?.dataObj));

if (!framerate || isNaN(framerate)) self.framerate = "24";
if (!framerate || Number.isNaN(framerate)) self.framerate = "24";
else if (framerate < 1) self.framerate = String(1 / framerate);
else self.framerate = String(framerate);
},
Expand Down Expand Up @@ -178,9 +178,9 @@ const Model = types
////// Incoming

registerSyncHandlers() {
["play", "pause", "seek"].forEach((event) => {
for (const event of ["play", "pause", "seek"]) {
self.syncHandlers.set(event, self.handleSync);
});
}
self.syncHandlers.set("speed", self.handleSyncSpeed);
},

Expand Down Expand Up @@ -260,9 +260,9 @@ const Model = types
const area = self.annotation.createResult({ sequence }, {}, control, self);

// add labels
self.activeStates().forEach((tag) => {
for (const tag of self.activeStates()) {
area.setValue(tag);
});
}

return area;
},
Expand Down Expand Up @@ -304,6 +304,9 @@ const Model = types
* @returns {Object} created region
*/
startDrawing({ frame, region: id }) {
// don't create or edit regions in read-only mode
if (self.annotation.isReadOnly()) return null;

if (id) {
const region = self.annotation.regions.find((r) => r.cleanId === id);
const range = region?.ranges?.[0];
Expand Down
Loading