microsoft
/
VoTT
зеркало из
1
0
Форкнуть 0
Код Задачи Пакеты Проекты Релизы Вики Активность

refactor:Refactor canvas component to utilize internal state (#594) 

Refactors the canvas component to utilize internal state as opposed to directly modifying the selected asset from the properties. Fixes some race conditions when making changes to assets and their regions.

[AB#17114]
This commit is contained in:
Tanner Barlow 2019-02-20 14:43:47 -08:00 коммит произвёл GitHub
Родитель aea056ea26
Коммит 54d5ca7b12
Не найден ключ, соответствующий данной подписи
Идентификатор ключа GPG: 4AEE18F83AFDEB23
4 изменённых файлов: 163 добавлений и 123 удалений
Показать статистику Скачать Patch файл Скачать Diff файл Показать все файлы Свернуть все файлы

14
src/common/mockFactory.ts
Просмотреть файл

@ -54,7 +54,7 @@ export default class MockFactory {
* @param assetType Type of asset
*/
public static createTestAsset(
name: string,
name: string = "test",
assetState: AssetState = AssetState.NotVisited,
path: string = `C:\\Desktop\\asset${name}.jpg`,
assetType: AssetType = AssetType.Image): IAsset {
@ -194,10 +194,10 @@ export default class MockFactory {
* Creates fake IAssetMetadata
* @param asset Test asset
*/
public static createTestAssetMetadata(asset: IAsset): IAssetMetadata {
public static createTestAssetMetadata(asset: IAsset, regions?: IRegion[]): IAssetMetadata {
return {
asset,
regions: [],
regions: regions || [],
};
}
@ -577,6 +577,14 @@ export default class MockFactory {
return new Canvas(canvasProps);
}
public static createTestRegions(count= 5) {
const regions: IRegion[] = [];
for (let i = 1; i <= count; i++) {
regions.push(MockFactory.createTestRegion(`test${i}`));
}
return regions;
}
public static createTestRegion(id = null) {
const testRegion: any = {
boundingBox: {

156
src/react/components/pages/editorPage/canvas.test.tsx
Просмотреть файл

@ -5,7 +5,7 @@ import { RegionData, RegionDataType } from "vott-ct/lib/js/CanvasTools/Core/Regi
import { Point2D } from "vott-ct/lib/js/CanvasTools/Core/Point2D";
import { AssetPreview, IAssetPreviewProps } from "../../common/assetPreview/assetPreview";
import MockFactory from "../../../../common/mockFactory";
import { EditorMode } from "../../../../models/applicationState";
import { EditorMode, IAssetMetadata } from "../../../../models/applicationState";
jest.mock("vott-ct/lib/js/CanvasTools/CanvasTools.Editor");
import { Editor } from "vott-ct/lib/js/CanvasTools/CanvasTools.Editor";
@ -15,8 +15,6 @@ import { RegionsManager } from "vott-ct/lib/js/CanvasTools/Region/RegionsManager
import { SelectionMode, AreaSelector } from "vott-ct/lib/js/CanvasTools/Selection/AreaSelector";
describe("Editor Canvas", () => {
let wrapper: ReactWrapper<ICanvasProps, ICanvasState, Canvas> = null;
const onAssetMetadataChanged = jest.fn();
function createTestRegionData() {
const testRegionData = new RegionData(0, 0, 100, 100,
@ -24,28 +22,34 @@ describe("Editor Canvas", () => {
return testRegionData;
}
function createComponent(canvasProps: ICanvasProps, assetPreviewProps: IAssetPreviewProps)
function createComponent(canvasProps?: ICanvasProps, assetPreviewProps?: IAssetPreviewProps)
: ReactWrapper<ICanvasProps, ICanvasState, Canvas> {
return mount(
<Canvas {...canvasProps}>
<AssetPreview {...assetPreviewProps} />
</Canvas>,
);
const props = createProps();
const cProps = canvasProps || props.canvas;
const aProps = assetPreviewProps || props.assetPreview;
return mount(
<Canvas {...cProps}>
<AssetPreview {...aProps} />
</Canvas>,
);
}
function getAssetMetadata() {
return MockFactory.createTestAssetMetadata(
MockFactory.createTestAsset(), MockFactory.createTestRegions());
}
function createProps() {
const assetMetadata = MockFactory.createTestAssetMetadata(MockFactory.createTestAsset("test"));
const canvasProps: ICanvasProps = {
selectedAsset: assetMetadata,
onAssetMetadataChanged,
selectedAsset: getAssetMetadata(),
onAssetMetadataChanged: jest.fn(),
editorMode: EditorMode.Rectangle,
selectionMode: SelectionMode.RECT,
project: MockFactory.createTestProject(),
};
const assetPreviewProps: IAssetPreviewProps = {
asset: assetMetadata.asset,
asset: getAssetMetadata().asset,
};
return {
@ -62,23 +66,22 @@ describe("Editor Canvas", () => {
editorMock.prototype.AS = {setSelectionMode: jest.fn()};
});
beforeEach(() => {
const props = createProps();
wrapper = createComponent(props.canvas, props.assetPreview);
});
it("renders correctly from default state", () => {
const wrapper = createComponent();
expect(wrapper.find(".canvas-enabled").exists()).toBe(true);
expect(wrapper.state()).toEqual({
contentSource: null,
selectedRegions: [],
canvasEnabled: true,
currentAsset: getAssetMetadata(),
});
expect(wrapper.instance().editor.RM.deleteAllRegions).toBeCalled();
});
it("regions are cleared and reset when selected asset changes", () => {
const wrapper = createComponent();
const rmMock = RegionsManager as any;
rmMock.prototype.deleteAllRegions.mockClear();
@ -92,6 +95,7 @@ describe("Editor Canvas", () => {
});
it("canvas is updated when asset loads", () => {
const wrapper = createComponent();
wrapper.find(AssetPreview).props().onLoaded(expect.any(HTMLImageElement));
expect(wrapper.instance().editor.addContentSource).toBeCalledWith(expect.any(HTMLImageElement));
@ -99,6 +103,7 @@ describe("Editor Canvas", () => {
});
it("canvas is enabled when an asset is deactivated", () => {
const wrapper = createComponent();
wrapper.find(AssetPreview).props().onDeactivated(expect.any(HTMLImageElement));
expect(wrapper.instance().editor.addContentSource).toBeCalledWith(expect.any(HTMLImageElement));
@ -106,22 +111,34 @@ describe("Editor Canvas", () => {
});
it("canvas is deactivated when an asset is activated", () => {
const wrapper = createComponent();
wrapper.find(AssetPreview).props().onActivated(expect.any(HTMLImageElement));
expect(wrapper.state().canvasEnabled).toEqual(false);
});
it("onSelectionEnd adds region to asset and selects it", () => {
const wrapper = createComponent();
const onAssetMetadataChanged = jest.fn();
wrapper.setProps({onAssetMetadataChanged});
const testCommit = createTestRegionData();
const canvas = wrapper.instance();
const testRegion = MockFactory.createTestRegion();
canvas.editor.onSelectionEnd(testCommit);
expect(onAssetMetadataChanged).toBeCalled();
expect(wrapper.prop("selectedAsset").regions).toMatchObject([testRegion]);
const testRegion = MockFactory.createTestRegion();
const originalAssetMetadata = getAssetMetadata();
expect(wrapper.instance().state.selectedRegions).toMatchObject([testRegion]);
expect(wrapper.state().currentAsset.regions).toMatchObject([
...originalAssetMetadata.regions,
testRegion,
]);
});
it("canvas updates regions when a new asset is loaded", async () => {
const wrapper = createComponent();
const assetMetadata = MockFactory.createTestAssetMetadata(MockFactory.createTestAsset("new-asset"));
assetMetadata.regions.push(MockFactory.createMockRegion());
assetMetadata.regions.push(MockFactory.createMockRegion());
@ -139,65 +156,94 @@ describe("Editor Canvas", () => {
});
it("onRegionMove edits region info in asset", () => {
const wrapper = createComponent();
const onAssetMetadataChanged = jest.fn();
wrapper.setProps({onAssetMetadataChanged});
const canvas = wrapper.instance();
const testRegion = MockFactory.createTestRegion("test-region");
testRegion.points = [new Point2D(0, 1), new Point2D(1, 1), new Point2D(0, 2), new Point2D(1, 2)];
wrapper.prop("selectedAsset").regions.push(testRegion);
canvas.editor.onRegionMoveEnd("test-region", createTestRegionData());
const regionData = createTestRegionData();
canvas.editor.onRegionMoveEnd("test1", regionData);
expect(onAssetMetadataChanged).toBeCalled();
expect(wrapper.prop("selectedAsset").regions).toMatchObject([MockFactory.createTestRegion("test-region")]);
const originalAssetMetadata = getAssetMetadata();
expect(onAssetMetadataChanged).toBeCalledWith({
...originalAssetMetadata,
regions: originalAssetMetadata.regions.map((r) => {
if (r.id === "test1") {
return {
...r,
points: regionData.points,
};
}
return r;
}),
});
});
it("onRegionDelete removes region from asset and clears selectedRegions", () => {
const wrapper = createComponent();
const onAssetMetadataChanged = jest.fn();
wrapper.setProps({onAssetMetadataChanged});
const originalAssetMetadata = getAssetMetadata();
expect(wrapper.state().currentAsset.regions.length).toEqual(originalAssetMetadata.regions.length);
const canvas = wrapper.instance();
const testRegion = MockFactory.createTestRegion("test-region");
canvas.editor.onRegionDelete("test1");
wrapper.prop("selectedAsset").regions.push(testRegion);
expect(wrapper.prop("selectedAsset").regions.length).toEqual(1);
canvas.editor.onRegionDelete("test-region");
expect(onAssetMetadataChanged).toBeCalled();
expect(wrapper.prop("selectedAsset").regions.length).toEqual(0);
expect(wrapper.state().currentAsset.regions.length).toEqual(originalAssetMetadata.regions.length - 1);
expect(onAssetMetadataChanged).toBeCalledWith({
...originalAssetMetadata,
regions: originalAssetMetadata.regions.filter((r) => r.id !== "test1"),
});
expect(wrapper.instance().state.selectedRegions.length).toEqual(0);
});
it("onRegionSelected adds region to list of selected regions on asset", () => {
const wrapper = createComponent();
const canvas = wrapper.instance();
const testRegion1 = MockFactory.createTestRegion("test1");
const testRegion2 = MockFactory.createTestRegion("test2");
wrapper.prop("selectedAsset").regions.push(testRegion1);
wrapper.prop("selectedAsset").regions.push(testRegion2);
expect(wrapper.prop("selectedAsset").regions.length).toEqual(2);
const originalAssetMetadata = getAssetMetadata();
expect(wrapper.state().currentAsset.regions.length).toEqual(originalAssetMetadata.regions.length);
canvas.editor.onRegionSelected("test1", false);
expect(wrapper.instance().state.selectedRegions.length).toEqual(1);
expect(wrapper.instance().state.selectedRegions)
expect(wrapper.state().selectedRegions.length).toEqual(1);
expect(wrapper.state().selectedRegions)
.toMatchObject([MockFactory.createTestRegion("test1")]);
canvas.editor.onRegionSelected("test2", true);
expect(wrapper.instance().state.selectedRegions.length).toEqual(2);
expect(wrapper.instance().state.selectedRegions)
expect(wrapper.state().selectedRegions.length).toEqual(2);
expect(wrapper.state().selectedRegions)
.toMatchObject([MockFactory.createTestRegion("test1"), MockFactory.createTestRegion("test2")]);
});
it("onTagClicked", () => {
it("Applies tag to selected region", () => {
const wrapper = createComponent();
const onAssetMetadataChanged = jest.fn();
wrapper.setProps({onAssetMetadataChanged});
const canvas = wrapper.instance();
const testRegion1 = MockFactory.createTestRegion("test1");
const testRegion2 = MockFactory.createTestRegion("test2");
wrapper.prop("selectedAsset").regions.push(testRegion1);
wrapper.prop("selectedAsset").regions.push(testRegion2);
canvas.editor.onRegionSelected("test1", false);
canvas.editor.onRegionSelected("test2", true);
canvas.editor.onRegionSelected("test1", null);
const newTag = MockFactory.createTestTag();
canvas.onTagClicked(newTag);
for (const region of wrapper.instance().state.selectedRegions) {
expect(region.tags.findIndex((tag) => tag === newTag.name)).toBeGreaterThanOrEqual(0);
}
canvas.applyTag(newTag.name);
const original = getAssetMetadata();
const expected: IAssetMetadata = {
...original,
regions: original.regions.map((r) => {
if (r.id === "test1") {
return {
...r,
tags: [newTag.name],
};
}
return r;
}),
};
expect(onAssetMetadataChanged).toBeCalledWith(expected);
expect(wrapper.state().currentAsset.regions[0].tags).toEqual([newTag.name]);
});
});

88
src/react/components/pages/editorPage/canvas.tsx
Просмотреть файл

@ -21,6 +21,7 @@ export interface ICanvasProps extends React.Props<Canvas> {
}
export interface ICanvasState {
currentAsset: IAssetMetadata;
contentSource: ContentSource;
selectedRegions?: IRegion[];
canvasEnabled: boolean;
@ -37,6 +38,7 @@ export default class Canvas extends React.Component<ICanvasProps, ICanvasState>
public editor: Editor;
public state: ICanvasState = {
currentAsset: this.props.selectedAsset,
contentSource: null,
selectedRegions: [],
canvasEnabled: true,
@ -65,9 +67,10 @@ export default class Canvas extends React.Component<ICanvasProps, ICanvasState>
public componentDidUpdate = (prevProps: Readonly<ICanvasProps>) => {
if (this.props.selectedAsset.asset.id !== prevProps.selectedAsset.asset.id) {
this.clearAllRegions();
if (this.props.selectedAsset.regions.length) {
this.updateSelected([]);
}
this.setState({
currentAsset: this.props.selectedAsset,
selectedRegions: [],
});
}
if (this.props.selectionMode !== prevProps.selectionMode) {
@ -91,12 +94,12 @@ export default class Canvas extends React.Component<ICanvasProps, ICanvasState>
}
/**
* Add tag to or remove tag from selected regions
* @param tag Tag to apply to or remove from selected regions
* Toggles tag on all selected regions
* @param selectedTag Tag name
*/
public onTagClicked = (tag: ITag) => {
public applyTag = (selectedTag: string) => {
for (const region of this.state.selectedRegions) {
this.toggleTagOnRegion(region, tag);
this.toggleTagOnRegion(region, selectedTag);
}
}
@ -124,15 +127,23 @@ export default class Canvas extends React.Component<ICanvasProps, ICanvasState>
},
points: scaledRegionData.points,
};
const currentAssetMetadata = this.props.selectedAsset;
currentAssetMetadata.regions.push(newRegion);
this.updateSelected([newRegion]);
this.updateAssetRegions([
...this.state.currentAsset.regions,
newRegion,
], [newRegion]);
}
if (currentAssetMetadata.regions.length) {
currentAssetMetadata.asset.state = AssetState.Tagged;
}
this.props.onAssetMetadataChanged(currentAssetMetadata);
private updateAssetRegions(regions: IRegion[], selectedRegions: IRegion[]) {
const currentAsset: IAssetMetadata = {
...this.state.currentAsset,
regions,
};
this.setState({
currentAsset,
selectedRegions,
}, () => {
this.props.onAssetMetadataChanged(currentAsset);
});
}
/**
@ -142,18 +153,17 @@ export default class Canvas extends React.Component<ICanvasProps, ICanvasState>
* @returns {void}
*/
private onRegionMoveEnd = (id: string, regionData: RegionData) => {
const currentAssetMetadata = this.props.selectedAsset;
const movedRegionIndex = currentAssetMetadata.regions.findIndex((region) => region.id === id);
const movedRegion = currentAssetMetadata.regions[movedRegionIndex];
const currentRegions = this.state.currentAsset.regions;
const movedRegionIndex = currentRegions.findIndex((region) => region.id === id);
const movedRegion = currentRegions[movedRegionIndex];
const scaledRegionData = this.editor.scaleRegionToSourceSize(regionData);
if (movedRegion) {
movedRegion.points = scaledRegionData.points;
}
currentAssetMetadata.regions[movedRegionIndex] = movedRegion;
this.updateSelected([movedRegion]);
this.props.onAssetMetadataChanged(currentAssetMetadata);
currentRegions[movedRegionIndex] = movedRegion;
this.updateAssetRegions(currentRegions, [movedRegion]);
}
/**
@ -166,16 +176,11 @@ export default class Canvas extends React.Component<ICanvasProps, ICanvasState>
this.editor.RM.deleteRegionById(id);
// Remove from project
const currentAssetMetadata = this.props.selectedAsset;
const deletedRegionIndex = this.props.selectedAsset.regions.findIndex((region) => region.id === id);
currentAssetMetadata.regions.splice(deletedRegionIndex, 1);
const currentRegions = this.state.currentAsset.regions;
const deletedRegionIndex = currentRegions.findIndex((region) => region.id === id);
currentRegions.splice(deletedRegionIndex, 1);
if (!currentAssetMetadata.regions.length) {
currentAssetMetadata.asset.state = AssetState.Visited;
}
this.props.onAssetMetadataChanged(currentAssetMetadata);
this.updateSelected([]);
this.updateAssetRegions(currentRegions, []);
}
/**
@ -185,16 +190,14 @@ export default class Canvas extends React.Component<ICanvasProps, ICanvasState>
* @returns {void}
*/
private onRegionSelected = (id: string, multiselect: boolean) => {
const region = this.state.currentAsset.regions.find((region) => region.id === id);
let selectedRegions = this.state.selectedRegions;
if (multiselect) {
selectedRegions.push(
this.props.selectedAsset.regions.find((region) => region.id === id));
selectedRegions.push(region);
} else {
selectedRegions = [
this.props.selectedAsset.regions.find((region) => region.id === id)];
selectedRegions = [region];
}
this.updateSelected(selectedRegions);
this.setState({selectedRegions});
}
private renderChildren = () => {
@ -270,9 +273,12 @@ export default class Canvas extends React.Component<ICanvasProps, ICanvasState>
* @param region Region to add or remove tag
* @param tag Tag to add or remove from region
*/
private toggleTagOnRegion = (region: IRegion, tag: ITag) => {
CanvasHelpers.toggleTag(region.tags, tag.name);
private toggleTagOnRegion = (region: IRegion, tag: string) => {
CanvasHelpers.toggleTag(region.tags, tag);
this.editor.RM.updateTagsById(region.id, CanvasHelpers.getTagsDescriptor(this.props.project.tags, region));
const updatedRegions = this.state.currentAsset.regions.map((r) => (r.id === region.id) ? region : r);
const updatedSelectedRegions = this.state.currentAsset.regions.map((r) => (r.id === region.id) ? region : r);
this.updateAssetRegions(updatedRegions, updatedSelectedRegions);
}
/**
@ -283,12 +289,12 @@ export default class Canvas extends React.Component<ICanvasProps, ICanvasState>
}
private updateRegions = () => {
if (!this.props.selectedAsset.regions || this.props.selectedAsset.regions.length === 0) {
if (!this.state.currentAsset.regions || this.state.currentAsset.regions.length === 0) {
return;
}
// Add regions to the canvas
this.props.selectedAsset.regions.forEach((region: IRegion) => {
this.state.currentAsset.regions.forEach((region: IRegion) => {
const loadedRegionData = CanvasHelpers.getRegionData(region);
this.editor.RM.addRegion(
region.id,
@ -298,7 +304,7 @@ export default class Canvas extends React.Component<ICanvasProps, ICanvasState>
// Set selected region to the last region
this.setState({
selectedRegions: [this.props.selectedAsset.regions[this.props.selectedAsset.regions.length - 1]],
selectedRegions: [this.state.currentAsset.regions[this.state.currentAsset.regions.length - 1]],
});
}

28
src/react/components/pages/editorPage/editorPage.tsx
Просмотреть файл

@ -54,6 +54,8 @@ export interface IEditorPageState {
childAssets?: IAsset[];
/** Additional settings for asset previews */
additionalSettings?: IAssetPreviewSettings;
/** Most recently selected tag */
selectedTag: string;
}
function mapStateToProps(state: IApplicationState) {
@ -77,6 +79,7 @@ function mapDispatchToProps(dispatch) {
export default class EditorPage extends React.Component<IEditorPageProps, IEditorPageState> {
public state: IEditorPageState = {
project: this.props.project,
selectedTag: null,
selectionMode: SelectionMode.RECT,
assets: [],
childAssets: [],
@ -181,30 +184,7 @@ export default class EditorPage extends React.Component<IEditorPageProps, IEdito
* @param tag Tag clicked
*/
public onTagClicked = (tag: ITag): void => {
const selectedAsset = this.state.selectedAsset;
if (this.canvas.current.state.selectedRegions && this.canvas.current.state.selectedRegions.length) {
const selectedRegions = this.canvas.current.state.selectedRegions;
selectedRegions.map((region) => {
const selectedIndex = selectedAsset.regions.findIndex((r) => r.id === region.id);
const selectedRegion = selectedAsset.regions[selectedIndex];
const tagIndex = selectedRegion.tags.findIndex((existingTag) => existingTag === tag.name);
if (tagIndex === -1) {
selectedRegion.tags.push(tag.name);
} else {
selectedRegion.tags.splice(tagIndex, 1);
}
if (selectedRegion.tags.length) {
this.canvas.current.editor.RM.updateTagsById(selectedRegion.id,
new TagsDescriptor(selectedRegion.tags.map((tempTag) => new Tag(tempTag,
this.props.project.tags.find((t) => t.name === tempTag).color))));
} else {
this.canvas.current.editor.RM.updateTagsById(selectedRegion.id, null);
}
return region;
});
}
this.onAssetMetadataChanged(selectedAsset);
this.canvas.current.applyTag(tag.name);
}
/**